Message ID | 1389743541.31367.279.camel@edumazet-glaptop2.roam.corp.google.com |
---|---|
State | RFC, archived |
Delegated to: | David Miller |
Headers | show |
On Tue, 2014-01-14 at 15:52 -0800, Eric Dumazet wrote: > Following code : > > if (unlikely(skb->len >= 2 && *(unsigned short *)(skb->data) == 0xFFFF)) > return htons(ETH_P_802_3); > > expects the additional bytes are in skb->head > > I do not think all eth_type_trans() are ready for a pskb_may_pull() > (I am too lazy to perform a whole check) > > Would following workaround be OK ? It seems to be an improvement, as LLC packets will no longer falsely be marked as Netware raw 802.3. You don't fix errors in the other direction, which should be more common, but the lack of bug reports about that suggests that no-one is using the old Netware protocol any more, or they're being very careful about which drivers they use. Ben. > diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c > index 8f032bae60ad..d95403098f09 100644 > --- a/net/ethernet/eth.c > +++ b/net/ethernet/eth.c > @@ -194,7 +194,8 @@ __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev) > * layer. We look for FFFF which isn't a used 802.2 SSAP/DSAP. This > * won't work for fault tolerant netware but does for the rest. > */ > - if (unlikely(skb->len >= 2 && *(unsigned short *)(skb->data) == 0xFFFF)) > + if (unlikely(skb_headlen(skb) >= 2 && > + *(unsigned short *)(skb->data) == 0xFFFF)) > return htons(ETH_P_802_3); > > /*
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Tue, 14 Jan 2014 15:52:21 -0800 > @@ -194,7 +194,8 @@ __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev) > * layer. We look for FFFF which isn't a used 802.2 SSAP/DSAP. This > * won't work for fault tolerant netware but does for the rest. > */ > - if (unlikely(skb->len >= 2 && *(unsigned short *)(skb->data) == 0xFFFF)) > + if (unlikely(skb_headlen(skb) >= 2 && > + *(unsigned short *)(skb->data) == 0xFFFF)) > return htons(ETH_P_802_3); I think using skb_header_pointer() will essentially have the same cost, why not use it instead? Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2014-01-15 at 15:48 -0800, David Miller wrote: > From: Eric Dumazet <eric.dumazet@gmail.com> > Date: Tue, 14 Jan 2014 15:52:21 -0800 > > > @@ -194,7 +194,8 @@ __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev) > > * layer. We look for FFFF which isn't a used 802.2 SSAP/DSAP. This > > * won't work for fault tolerant netware but does for the rest. > > */ > > - if (unlikely(skb->len >= 2 && *(unsigned short *)(skb->data) == 0xFFFF)) > > + if (unlikely(skb_headlen(skb) >= 2 && > > + *(unsigned short *)(skb->data) == 0xFFFF)) > > return htons(ETH_P_802_3); > > I think using skb_header_pointer() will essentially have the same cost, > why not use it instead? Yes, we can try this, I'll send a patch, thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c index 8f032bae60ad..d95403098f09 100644 --- a/net/ethernet/eth.c +++ b/net/ethernet/eth.c @@ -194,7 +194,8 @@ __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev) * layer. We look for FFFF which isn't a used 802.2 SSAP/DSAP. This * won't work for fault tolerant netware but does for the rest. */ - if (unlikely(skb->len >= 2 && *(unsigned short *)(skb->data) == 0xFFFF)) + if (unlikely(skb_headlen(skb) >= 2 && + *(unsigned short *)(skb->data) == 0xFFFF)) return htons(ETH_P_802_3); /*