* [PATCH v2 1/2] tg3: Work around HW/FW limitations with vlan encapsulated frames
2014-09-19 22:23 [PATCH v2 0/2] tg3: Fix handling of non-acceleration vlans Vladislav Yasevich
@ 2014-09-19 22:23 ` Vladislav Yasevich
2014-09-19 22:23 ` [PATCH v2 2/2] tg3: Allow for receive of full-size 8021AD frames Vladislav Yasevich
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Vladislav Yasevich @ 2014-09-19 22:23 UTC (permalink / raw)
To: netdev; +Cc: Vladislav Yasevich, Prashant Sreedharan, Michael Chan
TG3 appears to have an issue performing TSO and checksum offloading
correctly when the frame has been vlan encapsulated (non-accelerated).
In these cases, tcp checksum is not correctly updated.
This patch attempts to work around this issue. After the patch,
802.1ad vlans start working correctly over tg3 devices.
CC: Prashant Sreedharan <prashant@broadcom.com>
CC: Michael Chan <mchan@broadcom.com>
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
v2: - Moved segmentation call sooner
- Preserve checksum txd flag for LSO.
drivers/net/ethernet/broadcom/tg3.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index cb77ae9..b78d043 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -7924,6 +7924,13 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
if (skb_cow_head(skb, 0))
goto drop;
+ /* HW/FW can not correctly segment packets that have been
+ * vlan encapsulated.
+ */
+ if (skb->protocol == htons(ETH_P_8021Q) ||
+ skb->protocol == htons(ETH_P_8021AD))
+ return tg3_tso_bug(tp, tnapi, txq, skb);
+
iph = ip_hdr(skb);
tcp_opt_len = tcp_optlen(skb);
@@ -7979,6 +7986,16 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
base_flags |= tsflags << 12;
}
}
+ } else if (skb->ip_summed == CHECKSUM_PARTIAL) {
+ /* HW/FW can not correctly checksum packets that have been
+ * vlan encapsulated.
+ */
+ if (skb->protocol == htons(ETH_P_8021Q) ||
+ skb->protocol == htons(ETH_P_8021AD)) {
+ if (skb_checksum_help(skb))
+ goto drop;
+ base_flags &= ~TXD_FLAG_TCPUDP_CSUM;
+ }
}
if (tg3_flag(tp, USE_JUMBO_BDFLAG) &&
--
1.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 2/2] tg3: Allow for receive of full-size 8021AD frames
2014-09-19 22:23 [PATCH v2 0/2] tg3: Fix handling of non-acceleration vlans Vladislav Yasevich
2014-09-19 22:23 ` [PATCH v2 1/2] tg3: Work around HW/FW limitations with vlan encapsulated frames Vladislav Yasevich
@ 2014-09-19 22:23 ` Vladislav Yasevich
2014-09-19 22:59 ` Eric Dumazet
2014-09-20 0:39 ` [PATCH v2 0/2] tg3: Fix handling of non-acceleration vlans Prashant Sreedharan
2014-09-30 18:53 ` [PATCH] tg3: Allow for receive of full-size 8021AD frames Vlad Yasevich
3 siblings, 1 reply; 9+ messages in thread
From: Vladislav Yasevich @ 2014-09-19 22:23 UTC (permalink / raw)
To: netdev; +Cc: Vladislav Yasevich, Prashant Sreedharan, Michael Chan
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;
}
--
1.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2 2/2] tg3: Allow for receive of full-size 8021AD frames
2014-09-19 22:23 ` [PATCH v2 2/2] tg3: Allow for receive of full-size 8021AD frames Vladislav Yasevich
@ 2014-09-19 22:59 ` Eric Dumazet
2014-09-19 23:14 ` Prashant Sreedharan
0 siblings, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2014-09-19 22:59 UTC (permalink / raw)
To: Vladislav Yasevich
Cc: netdev, Vladislav Yasevich, Prashant Sreedharan, Michael Chan
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 ?
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2 2/2] tg3: Allow for receive of full-size 8021AD frames
2014-09-19 22:59 ` Eric Dumazet
@ 2014-09-19 23:14 ` Prashant Sreedharan
2014-09-19 23:37 ` Eric Dumazet
0 siblings, 1 reply; 9+ messages in thread
From: Prashant Sreedharan @ 2014-09-19 23:14 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Vladislav Yasevich, netdev, Vladislav Yasevich, Michael Chan
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.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2 2/2] tg3: Allow for receive of full-size 8021AD frames
2014-09-19 23:14 ` Prashant Sreedharan
@ 2014-09-19 23:37 ` Eric Dumazet
0 siblings, 0 replies; 9+ messages in thread
From: Eric Dumazet @ 2014-09-19 23:37 UTC (permalink / raw)
To: Prashant Sreedharan
Cc: Vladislav Yasevich, netdev, Vladislav Yasevich, Michael Chan
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 ;)
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/2] tg3: Fix handling of non-acceleration vlans
2014-09-19 22:23 [PATCH v2 0/2] tg3: Fix handling of non-acceleration vlans Vladislav Yasevich
2014-09-19 22:23 ` [PATCH v2 1/2] tg3: Work around HW/FW limitations with vlan encapsulated frames Vladislav Yasevich
2014-09-19 22:23 ` [PATCH v2 2/2] tg3: Allow for receive of full-size 8021AD frames Vladislav Yasevich
@ 2014-09-20 0:39 ` Prashant Sreedharan
2014-09-30 18:53 ` [PATCH] tg3: Allow for receive of full-size 8021AD frames Vlad Yasevich
3 siblings, 0 replies; 9+ messages in thread
From: Prashant Sreedharan @ 2014-09-20 0:39 UTC (permalink / raw)
To: Vladislav Yasevich; +Cc: netdev, Vladislav Yasevich, Michael Chan
On Fri, 2014-09-19 at 18:23 -0400, Vladislav Yasevich wrote:
> TG3 can't cope with checksum and TSO offloads when vlan headers were
> not accelerated. This can be demonstrated with 802.1ad vlans or
> by configuring a vlan on top of a bridge and turning off vlan acceleration
> on the bridge device.
>
> Instead of disabling all vlan acceleration, this series works around
> the issue by having tg3 driver call software segmentation and checksum
> generation.
>
> v2: - moved the call to software segment soonner
> - Preserve the checksum flags for TSO/LSO case and turn it off
> if software checksum was computed.
> - Add code to correctly receive full sized 802.1ad frames.
>
> Vladislav Yasevich (2):
> tg3: Work around HW/FW limitations with vlan encapsulated frames
> tg3: Allow for receive of full-size 8021AD frames
>
> drivers/net/ethernet/broadcom/tg3.c | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
Acked-by: Prashant Sreedharan <prashant@broadcom.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] tg3: Allow for receive of full-size 8021AD frames
2014-09-19 22:23 [PATCH v2 0/2] tg3: Fix handling of non-acceleration vlans Vladislav Yasevich
` (2 preceding siblings ...)
2014-09-20 0:39 ` [PATCH v2 0/2] tg3: Fix handling of non-acceleration vlans Prashant Sreedharan
@ 2014-09-30 18:53 ` Vlad Yasevich
2014-09-30 21:02 ` David Miller
3 siblings, 1 reply; 9+ messages in thread
From: Vlad Yasevich @ 2014-09-30 18:53 UTC (permalink / raw)
To: netdev; +Cc: Vladislav Yasevich, Prashant Sreedharan, Michael Chan
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>
Acked-by: Prashant Sreedharan <prashant@broadcom.com>
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
This patch somehow got dropped. Please apply and queue for stable.
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;
}
--
1.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread