On Sat, Nov 7, 2015 at 8:11 AM, Felix Fietkau [off-list ref] wrote:

On 2015-07-31 00:15, Martin Blumenstingl wrote:

quoted

On Wed, Jul 29, 2015 at 8:05 AM, Willem de Bruijn [off-list ref] wrote:

quoted

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.

quoted

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}