Syslinux - Jul 2015 - [PATCH] Updated udp.c to use real client ip and subnetmask values if on local subnet

On Wed, Jul 8, 2015 at 7:34 PM, Gene Cumm <gene.cumm at gmail.com> wrote:
> What about the observed symptoms?  Have you performed an inline packet
Jeff, there's specific code that creates specific responses that if
you had mentioned what you saw, I'd expect we could have seen
something sooner.

-- 
-Gene

On Fri, Jul 17, 2015 at 8:53 PM, Gene Cumm <gene.cumm at gmail.com>
wrote:
> On Wed, Jul 8, 2015 at 7:34 PM, Gene Cumm <gene.cumm at gmail.com>
wrote:
>
>> What about the observed symptoms?  Have you performed an inline packet
>
> Jeff, there's specific code that creates specific responses that if
> you had mentioned what you saw, I'd expect we could have seen
> something sooner.
Jeff, Patrick: Could you try my code from my github repo branch
efi-multinic?  It's derived from Patrick's code and I finally see good
responses with a VMware VM's e1000e NIC (never saw ANYTHING good from
it until now).

git://github.com/geneC/syslinux.git
https://github.com/geneC/syslinux.git

-- 
-Gene


-- 
-Gene

>>>
 Jeff, Patrick: Could you try my code from my github repo branch
 efi-multinic?? It's derived from Patrick's code and I finally see good
 responses with a VMware VM's e1000e NIC (never saw ANYTHING good from
 it until now).
 
 git://github.com/geneC/syslinux.git
 https://github.com/geneC/syslinux.git
 
 -- 
 -Gene
 <<<

I'll take a look at it;
BTW what was the problem you found? what did you change?
I'm using my code with VMware VM's e1000e NIC all the time w/o any
problem.

Best,
Patrick

>>>
 
 Jeff,
 Patrick: Could you try my code from my github repo branch
 efi-multinic?? It's derived from
 Patrick's code and I finally see good
 responses with a VMware VM's e1000e NIC
 (never saw ANYTHING good from
 it until
 now).
 
 git://github.com/geneC/syslinux.git
 https://github.com/geneC/syslinux.git
 
 -- 
 -Gene
 <<<<

I've seen your code.
Your first patch considered Pxebc, UDPv4Sb and  TCPv4Sb are created under the
same
handle but now we know this is not the case.
Your new patch now considers UDPv4Sb and TCPv4Sb are created under the same 
handle thing that we really do not know if is always true or not. 

My patch looks slower than yours because it parses the handles on every attempt
of binding
but it does not make any assumptions about the handles owning SB protocols. I
consider this
approach much safer.
About the speed of my patch; I do not think that matters compared to the time of
the smallest
transfer associated to the created child...
If you are really bothered by parsing the handles every time then we should save
both;
the associated UDPv4Sb and TCPv4Sb handles considering they might be different.

Best,
Patrick

>Jeff, there's specific code that creates specific responses that if
>you had mentioned what you saw, I'd expect we could have seen
>something sooner.
I promise to get to the heart of the matter quickly and provide as much 
real detail as possible from now on!

This week we are making our annual pilgrimage to our corporate 
headquarters so I will have limited access to a network. I will try to get 
as many answers as I can but the answers may come slower than normal.

Jeff

>>>
 Jeff, Patrick: Could you try my code from my github repo branch
 efi-multinic?? It's derived from Patrick's code and I finally see good
 responses with a VMware VM's e1000e NIC (never saw ANYTHING good from
 it until now).
 
 git://github.com/geneC/syslinux.git
 https://github.com/geneC/syslinux.git
 
 -- 
 -Gene
 <<<

Hi there
I think in the case of a particular driver that implements Pxebc but it does not
implement UDPv4Sb/TCPv4Sb
(HP Elitebook 2560p/8460p) your patch crashes;

In that case the following for(;;) structure will never have a MAC match, then
the
BS->OpenProtocol will never fail returning the needed "status !=
EFI_SUCCESS"
to avoid de-referencing an undefined pointer (sbp->CreateChild) in the next
code line.

...
for (i = 0; i < nr_handles; i++) {
	    DevicePath = DevicePathFromHandle(handles[i]);
	    if (efi_get_MAC(DevicePath, &mac_2, PXE_MAC_LENGTH)
		    && memcmp(mac_1, mac_2, PXE_MAC_LENGTH) == 0) {
		 sb_handle = handles[i];
		status = uefi_call_wrapper(BS->OpenProtocol, 6, sb_handle,
					bguid, (void **)&sbp,
					image_handle, sb_handle,
					EFI_OPEN_PROTOCOL_GET_PROTOCOL);
		if (status == EFI_SUCCESS) {
		    mnpsb_handle = sb_handle;
		    break;
		}
	    }

	}

 }
    if (status != EFI_SUCCESS)
       goto free_binding;

    child = NULL;

    status = uefi_call_wrapper(sbp->CreateChild, 2, sbp, (EFI_HANDLE
*)&child);  <<<< Potential crash here !

...


I think these changes would solve the thing.

...
-EFI_SERVICE_BINDING *sbp;
+EFI_SERVICE_BINDING *sbp =NULL;
...

-    if (status != EFI_SUCCESS)
+    if (status != EFI_SUCCESS || sbp != NULL)
       goto free_binding;
...


Please take a look;
BTW It's just only me or the adopted Linux kernel coding style makes
following C code really a PITA ??

Best,
Patrick