On Sat, Nov 7, 2015 at 8:11 AM, Felix Fietkau <n...@openwrt.org> wrote: > On 2015-07-31 00:15, Martin Blumenstingl wrote: >> On Wed, Jul 29, 2015 at 8:05 AM, Willem de Bruijn <will...@google.com> wrote: >>> Martin, to return to your initial statement that PPPoE PADI packets can >>> have a zero payload: the PPPoE RFC states that PADI packets "MUST >>> contain exactly one TAG of TAG_TYPE Service-Name, indicating the >>> service the Host is requesting, and any number of other TAG types." >>> (RFC 2516, 5.1). Is the observed behavior (no payload) perhaps >>> incorrect? >> As far as I can see you are right, but the real world seems to be different. >> My ISP for example lists the PPPoE connection settings, but they are >> nowhere mentioning the "service name". >> >> I have also re-read pppd's source code again and that seems to confirm >> what you are reading in the RFC: Leaving the service name away makes >> seems to violate the RFC, but pppd still accepts those configurations. >> >>> Even if it is, if this is breaking established userspace expectations, >>> we should look into it. Ethernet specifies a minimum payload size of >>> 46 on the wire, but perhaps that is handled with padding, so that >>> 0 length should be valid within the stack. Also, there may be other >>> valid uses of 0 length payload on top of link layers that are not Ethernet. >> Good catch. I would also like to note that the documentation for >> "hard_header_len" describes it as "Hardware header length". When the >> purpose of this field we should check whether the documentation should >> be updated to "Minimum hardware header length" -> that would mean the >> condition has to be a "len < hard_header_len" instead of a "len <= >> hard_header_len" (as it is now). >> >> PS: I have also added the pppd maintainer (Paul Mackerras) to this >> thread because I think he should know about this issue (and he can >> probably provide more details if required). >> As a quick summary for him: linux >= 3.19 rejects PADI packets when >> no service name is configured. > Any news on this? Users are complaining about this regression: > https://dev.openwrt.org/ticket/20707
I took another look. This hinges on the question what the contract with device drivers is on skb network data and length. Is passing an skb with skb->len == 0 to ndo_start_xmit allowed? >From what I gather from the ethernet spec [1], sending frames with an empty head is allowed on that medium, at least. A quick scan of a few drivers and the loopback path also does not show anything that would break. In some cases, skb_network_header points beyond the end of the buffer (ETH_HLEN), but the length is correctly reported as 0. The tap device can also generate packets consisting of only a link layer header: compares len < ETH_HLEN in tun_get_user. So, I think that this change should be correct: static bool ll_header_truncated(const struct net_device *dev, int len) { - /* net device doesn't like empty head */ - if (unlikely(len <= dev->hard_header_len)) { + if (unlikely(len < dev->hard_header_len)) { but a definitive answer would require an audit of all device drivers (including bonding, ..) or at least the certainty that it has always been correct to send a packet of only link layer header to ndo_start_xmit. [1] IEEE 802.3™-2012 – Section One, {3.2.8, 4.2.3.3} -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html