| Message ID | 1411165389-21276-3-git-send-email-vyasevic@redhat.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Fri, 2014-09-19 at 18:23 -0400, Vladislav Yasevich wrote: > When receiving a vlan-tagged frame that still contains > a vlan header, the length of the packet will be greater > then MTU+ETH_HLEN since it will account of the extra > vlan header. TG3 checks this for the case for 802.1Q, > but not for 802.1ad. As a result, full sized 802.1ad > frames get dropped by the card. > > Add a check for 802.1ad protocol when receiving full > sized frames. > > Suggested-by: Prashant Sreedharan <prashant@broadcom.com> > CC: Prashant Sreedharan <prashant@broadcom.com> > CC: Michael Chan <mchan@broadcom.com> > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > --- > drivers/net/ethernet/broadcom/tg3.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c > index 0a0938d..4f674f9 100644 > --- a/drivers/net/ethernet/broadcom/tg3.c > +++ b/drivers/net/ethernet/broadcom/tg3.c > @@ -6918,7 +6918,8 @@ static int tg3_rx(struct tg3_napi *tnapi, int budget) > skb->protocol = eth_type_trans(skb, tp->dev); > > if (len > (tp->dev->mtu + ETH_HLEN) && > - skb->protocol != htons(ETH_P_8021Q)) { > + skb->protocol != htons(ETH_P_8021Q) && > + skb->protocol != htons(ETH_P_8021AD)) { > dev_kfree_skb_any(skb); > goto drop_it_no_recycle; > } Really I do not understand what is the value of this check. If NIC is dumb enough to send oversized frames, what prevents these frames being VLAN ones, and this test wont avoid the crash anyway ? -- 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 Fri, 2014-09-19 at 15:59 -0700, Eric Dumazet wrote: > On Fri, 2014-09-19 at 18:23 -0400, Vladislav Yasevich wrote: > > When receiving a vlan-tagged frame that still contains > > a vlan header, the length of the packet will be greater > > then MTU+ETH_HLEN since it will account of the extra > > vlan header. TG3 checks this for the case for 802.1Q, > > but not for 802.1ad. As a result, full sized 802.1ad > > frames get dropped by the card. > > > > Add a check for 802.1ad protocol when receiving full > > sized frames. > > > > Suggested-by: Prashant Sreedharan <prashant@broadcom.com> > > CC: Prashant Sreedharan <prashant@broadcom.com> > > CC: Michael Chan <mchan@broadcom.com> > > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > > --- > > drivers/net/ethernet/broadcom/tg3.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c > > index 0a0938d..4f674f9 100644 > > --- a/drivers/net/ethernet/broadcom/tg3.c > > +++ b/drivers/net/ethernet/broadcom/tg3.c > > @@ -6918,7 +6918,8 @@ static int tg3_rx(struct tg3_napi *tnapi, int budget) > > skb->protocol = eth_type_trans(skb, tp->dev); > > > > if (len > (tp->dev->mtu + ETH_HLEN) && > > - skb->protocol != htons(ETH_P_8021Q)) { > > + skb->protocol != htons(ETH_P_8021Q) && > > + skb->protocol != htons(ETH_P_8021AD)) { > > dev_kfree_skb_any(skb); > > goto drop_it_no_recycle; > > } > > Really I do not understand what is the value of this check. > > If NIC is dumb enough to send oversized frames, what prevents these > frames being VLAN ones, and this test wont avoid the crash anyway ? > The NIC will not receive frames of size > mtu + ETH_HLEN + VLAN_HLEN it will update the oversize_frame count and drop it. The check is to make sure the additional 4 bytes is due to a valid vlan header. -- 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 Fri, 2014-09-19 at 16:14 -0700, Prashant Sreedharan wrote: > The NIC will not receive frames of size > mtu + ETH_HLEN + VLAN_HLEN it > will update the oversize_frame count and drop it. The check is to make > sure the additional 4 bytes is due to a valid vlan header. I see, thanks for the explanation ;) -- 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/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c index 0a0938d..4f674f9 100644 --- a/drivers/net/ethernet/broadcom/tg3.c +++ b/drivers/net/ethernet/broadcom/tg3.c @@ -6918,7 +6918,8 @@ static int tg3_rx(struct tg3_napi *tnapi, int budget) skb->protocol = eth_type_trans(skb, tp->dev); if (len > (tp->dev->mtu + ETH_HLEN) && - skb->protocol != htons(ETH_P_8021Q)) { + skb->protocol != htons(ETH_P_8021Q) && + skb->protocol != htons(ETH_P_8021AD)) { dev_kfree_skb_any(skb); goto drop_it_no_recycle; }
When receiving a vlan-tagged frame that still contains a vlan header, the length of the packet will be greater then MTU+ETH_HLEN since it will account of the extra vlan header. TG3 checks this for the case for 802.1Q, but not for 802.1ad. As a result, full sized 802.1ad frames get dropped by the card. Add a check for 802.1ad protocol when receiving full sized frames. Suggested-by: Prashant Sreedharan <prashant@broadcom.com> CC: Prashant Sreedharan <prashant@broadcom.com> CC: Michael Chan <mchan@broadcom.com> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> --- drivers/net/ethernet/broadcom/tg3.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)