ping
On Thu, Apr 21, 2016 at 2:48 PM, Ladi Prosek <lpro...@redhat.com> wrote: > On Thu, Apr 21, 2016 at 12:18 PM, Michael Brown <mc...@ipxe.org> wrote: >> On 21/04/16 09:36, Ladi Prosek wrote: >>> >>> Following up on our IRC conversation, here's what I get when playing >>> with a Linux kernel with no VLAN support (!CONFIG_VLAN_8021Q && >>> !CONFIG_VLAN_8021Q_MODULE). >>> >>> Receiving VLAN 0 packets works. The tag is stripped and the packet >>> passes through the networking stack as usual. Here's a VLAN 0 ping >>> request and response, 10.34.1.105 is running the VLAN-less kernel: >>> >>> 1 0.000000 10.34.1.136 -> 10.34.1.105 ICMP 50 Echo (ping) >>> request id=0x03e8, seq=0/0, ttl=255 >>> 2 0.000080 10.34.1.105 -> 10.34.1.136 ICMP 46 Echo (ping) reply >>> id=0x03e8, seq=0/0, ttl=64 (request in 1) >>> >>> Note that the response is 4 bytes shorter because it doesn't have the >>> 802.1Q header. >> >> >> Fantastic; thank you for doing this testing. I am suitably convinced that >> we ought to treat packets on VLAN 0 in the same way as packets with no VLAN >> tag. >> >> Unfortunately, this is going to be difficult to do. Your patch handles >> packets that get processed by iPXE, but doesn't help with packets passed via >> the UNDI API. >> >> We do allow for a PXE NBP to use the UNDI API on a VLAN device. Only the >> selected VLAN device gets frozen (via netdev_freeze()). Packets arriving on >> the unfrozen trunk device are processed by iPXE as normal: the VLAN tag is >> stripped and the packet is requeued on the appropriately frozen VLAN device, >> whence pxenv_undi_isr() can dequeue it. >> >> This model breaks if we have to handle VLAN 0. There is no separate VLAN >> device, so the trunk device itself will be frozen. The packets will >> therefore end up being passed out directly via PXENV_UNDI_ISR; the VLAN tag >> will never be inspected or removed. This will break most PXE NBPs. >> >> I can't immediately see a clean way to do this. Potentially we would need >> to parse and strip the VLAN header at the point that netdev_rx() is called, >> instead of treating VLANs as a net_protocol. > > Interesting, thanks. Why not strip it inside pxenv_undi_isr then? This > function already peeks into the network layer protocol so it doesn't > feel that unnatural to special case 802.1Q. Maybe without the goto :) > vlan_strip would be the guts of vlan_rx factored out to a separate > function doing pretty much everything except re-enquing the packet. > > @@ -967,6 +968,7 @@ static PXENV_EXIT_t pxenv_undi_isr ( struct > s_PXENV_UNDI_ISR *undi_isr ) { > > /* Strip link-layer header */ > ll_protocol = pxe_netdev->ll_protocol; > + strip_ll_header: > if ( ( rc = ll_protocol->pull ( pxe_netdev, iobuf, &ll_dest, > &ll_source, &net_proto, > &flags ) ) != 0 ) { > @@ -978,6 +980,13 @@ static PXENV_EXIT_t pxenv_undi_isr ( struct > s_PXENV_UNDI_ISR *undi_isr ) { > > /* Determine network-layer protocol */ > switch ( net_proto ) { > + case htons ( ETH_P_8021Q ): > + if ( ( rc = vlan_strip ( pxe_netdev, iobuf, &ll_dest, > + &ll_source ) ) != 0 ) { > + undi_isr->Status = PXENV_STATUS_FAILURE; > + return PXENV_EXIT_FAILURE; > + } > + goto strip_ll_header; > case htons ( ETH_P_IP ): > net_protocol = &ipv4_protocol; > prottype = P_IP; _______________________________________________ ipxe-devel mailing list ipxe-devel@lists.ipxe.org https://lists.ipxe.org/mailman/listinfo.cgi/ipxe-devel