* [PATCHv2-net-3.8 0/3] fix kernel crash with macvtap on top of LRO
@ 2013-02-07 13:12 ` Michael S. Tsirkin
0 siblings, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2013-02-07 13:12 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, e1000-devel, Jitendra Kalsaria, Bruce Allan,
Jesse Brandeburg, Eilon Greenstein, John Fastabend, John Ronciak,
Sony Chacko, linux-driver, bhutchings, linux-kernel, Jacob Keller
At the moment, macvtap crashes are observed if macvtap is attached
to an interface with LRO enabled.
The crash in question is BUG() in macvtap_skb_to_vnet_hdr.
This happens because several drivers set gso_size but not gso_type
in incoming skbs.
This didn't use to be the case: with intel cards on 3.2 and older
kernels, with qlogic - on 3.4 and older kernels, so it's a regression if
not a recent one.
The following patches fix this for qlogic, broadcom and intel drivers.
I tested that the patch fixes the crash for ixgbe but
don't have qlogic/broadcom hardware to test.
I also only tested TCPv4.
Please review, and consider for 3.8.
Changes from v1:
- added missing htons as suggested by Eric
- backported the relevant bits from
cbf1de72324a8105ddcc3d9ce9acbc613faea17e for bnx2x
Michael S. Tsirkin (3):
ixgbe: fix gso type
qlcnic: set gso_type
bnx2x: set gso_type
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 12 +++++-------
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 8 ++++++--
drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c | 7 ++++++-
3 files changed, 17 insertions(+), 10 deletions(-)
--
MST
------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCHv2-net-3.8 1/3] ixgbe: fix gso type
2013-02-07 13:12 ` Michael S. Tsirkin
@ 2013-02-07 13:13 ` Michael S. Tsirkin
-1 siblings, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2013-02-07 13:13 UTC (permalink / raw)
To: netdev
Cc: Eilon Greenstein, Jeff Kirsher, Jesse Brandeburg, Bruce Allan,
Carolyn Wyborny, Don Skidmore, Greg Rose, Peter P Waskiewicz Jr,
Alex Duyck, John Ronciak, Tushar Dave, Jitendra Kalsaria,
Sony Chacko, linux-driver, John Fastabend, David S. Miller,
Jacob Keller, linux-kernel, e1000-devel, bhutchings, eric.dumazet
ixgbe set gso_size but not gso_type. This leads to
crashes in macvtap.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 20a5af6..e1b2d22 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1401,6 +1401,10 @@ static void ixgbe_set_rsc_gso_size(struct ixgbe_ring *ring,
/* set gso_size to avoid messing up TCP MSS */
skb_shinfo(skb)->gso_size = DIV_ROUND_UP((skb->len - hdr_len),
IXGBE_CB(skb)->append_cnt);
+ if (skb->protocol == __constant_htons(ETH_P_IPV6))
+ skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
+ else
+ skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
}
static void ixgbe_update_rsc_stats(struct ixgbe_ring *rx_ring,
@@ -1435,6 +1439,8 @@ static void ixgbe_process_skb_fields(struct ixgbe_ring *rx_ring,
{
struct net_device *dev = rx_ring->netdev;
+ skb->protocol = eth_type_trans(skb, dev);
+
ixgbe_update_rsc_stats(rx_ring, skb);
ixgbe_rx_hash(rx_ring, rx_desc, skb);
@@ -1450,8 +1456,6 @@ static void ixgbe_process_skb_fields(struct ixgbe_ring *rx_ring,
}
skb_record_rx_queue(skb, rx_ring->queue_index);
-
- skb->protocol = eth_type_trans(skb, dev);
}
static void ixgbe_rx_skb(struct ixgbe_q_vector *q_vector,
--
MST
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCHv2-net-3.8 1/3] ixgbe: fix gso type
@ 2013-02-07 13:13 ` Michael S. Tsirkin
0 siblings, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2013-02-07 13:13 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, e1000-devel, Jitendra Kalsaria, Bruce Allan,
Jesse Brandeburg, Eilon Greenstein, John Fastabend, John Ronciak,
Sony Chacko, linux-driver, bhutchings, linux-kernel, Jacob Keller
ixgbe set gso_size but not gso_type. This leads to
crashes in macvtap.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 20a5af6..e1b2d22 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1401,6 +1401,10 @@ static void ixgbe_set_rsc_gso_size(struct ixgbe_ring *ring,
/* set gso_size to avoid messing up TCP MSS */
skb_shinfo(skb)->gso_size = DIV_ROUND_UP((skb->len - hdr_len),
IXGBE_CB(skb)->append_cnt);
+ if (skb->protocol == __constant_htons(ETH_P_IPV6))
+ skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
+ else
+ skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
}
static void ixgbe_update_rsc_stats(struct ixgbe_ring *rx_ring,
@@ -1435,6 +1439,8 @@ static void ixgbe_process_skb_fields(struct ixgbe_ring *rx_ring,
{
struct net_device *dev = rx_ring->netdev;
+ skb->protocol = eth_type_trans(skb, dev);
+
ixgbe_update_rsc_stats(rx_ring, skb);
ixgbe_rx_hash(rx_ring, rx_desc, skb);
@@ -1450,8 +1456,6 @@ static void ixgbe_process_skb_fields(struct ixgbe_ring *rx_ring,
}
skb_record_rx_queue(skb, rx_ring->queue_index);
-
- skb->protocol = eth_type_trans(skb, dev);
}
static void ixgbe_rx_skb(struct ixgbe_q_vector *q_vector,
--
MST
------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCHv2-net-3.8 2/3] qlcnic: set gso_type
2013-02-07 13:12 ` Michael S. Tsirkin
@ 2013-02-07 13:13 ` Michael S. Tsirkin
-1 siblings, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2013-02-07 13:13 UTC (permalink / raw)
To: netdev
Cc: Eilon Greenstein, Jeff Kirsher, Jesse Brandeburg, Bruce Allan,
Carolyn Wyborny, Don Skidmore, Greg Rose, Peter P Waskiewicz Jr,
Alex Duyck, John Ronciak, Tushar Dave, Jitendra Kalsaria,
Sony Chacko, linux-driver, John Fastabend, David S. Miller,
Jacob Keller, linux-kernel, e1000-devel, bhutchings, eric.dumazet
qlcnic set gso_size but not gso type. This leads to crashes
in macvtap.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
index 6f82812..09aa310 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
@@ -986,8 +986,13 @@ qlcnic_process_lro(struct qlcnic_adapter *adapter,
th->seq = htonl(seq_number);
length = skb->len;
- if (adapter->flags & QLCNIC_FW_LRO_MSS_CAP)
+ if (adapter->flags & QLCNIC_FW_LRO_MSS_CAP) {
skb_shinfo(skb)->gso_size = qlcnic_get_lro_sts_mss(sts_data1);
+ if (skb->protocol == htons(ETH_P_IPV6))
+ skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
+ else
+ skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
+ }
if (vid != 0xffff)
__vlan_hwaccel_put_tag(skb, vid);
--
MST
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCHv2-net-3.8 2/3] qlcnic: set gso_type
@ 2013-02-07 13:13 ` Michael S. Tsirkin
0 siblings, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2013-02-07 13:13 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, e1000-devel, Jitendra Kalsaria, Bruce Allan,
Jesse Brandeburg, Eilon Greenstein, John Fastabend, John Ronciak,
Sony Chacko, linux-driver, bhutchings, linux-kernel, Jacob Keller
qlcnic set gso_size but not gso type. This leads to crashes
in macvtap.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
index 6f82812..09aa310 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
@@ -986,8 +986,13 @@ qlcnic_process_lro(struct qlcnic_adapter *adapter,
th->seq = htonl(seq_number);
length = skb->len;
- if (adapter->flags & QLCNIC_FW_LRO_MSS_CAP)
+ if (adapter->flags & QLCNIC_FW_LRO_MSS_CAP) {
skb_shinfo(skb)->gso_size = qlcnic_get_lro_sts_mss(sts_data1);
+ if (skb->protocol == htons(ETH_P_IPV6))
+ skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
+ else
+ skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
+ }
if (vid != 0xffff)
__vlan_hwaccel_put_tag(skb, vid);
--
MST
------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCHv2-net-3.8 2/3] qlcnic: set gso_type
2013-02-07 13:13 ` Michael S. Tsirkin
@ 2013-02-07 17:08 ` Jitendra Kalsaria
-1 siblings, 0 replies; 15+ messages in thread
From: Jitendra Kalsaria @ 2013-02-07 17:08 UTC (permalink / raw)
To: Michael S. Tsirkin, netdev
Cc: Eilon Greenstein, Jeff Kirsher, Jesse Brandeburg, Bruce Allan,
Carolyn Wyborny, Don Skidmore, Greg Rose, Peter P Waskiewicz Jr,
Alex Duyck, John Ronciak, Tushar Dave, Sony Chacko,
Dept-Eng Linux Driver, John Fastabend, David Miller, Jacob Keller,
linux-kernel, e1000-devel@lists.sourceforge.net,
bhutchings@solarflare.com, eric.dumazet@gmail.com
On 2/7/13 5:13 AM, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>qlcnic set gso_size but not gso type. This leads to crashes
>in macvtap.
>
>Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>---
> drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
>b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
>index 6f82812..09aa310 100644
>--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
>+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
>@@ -986,8 +986,13 @@ qlcnic_process_lro(struct qlcnic_adapter *adapter,
> th->seq = htonl(seq_number);
> length = skb->len;
>
>- if (adapter->flags & QLCNIC_FW_LRO_MSS_CAP)
>+ if (adapter->flags & QLCNIC_FW_LRO_MSS_CAP) {
> skb_shinfo(skb)->gso_size = qlcnic_get_lro_sts_mss(sts_data1);
>+ if (skb->protocol == htons(ETH_P_IPV6))
>+ skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
>+ else
>+ skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
>+ }
>
> if (vid != 0xffff)
> __vlan_hwaccel_put_tag(skb, vid);
>--
>MST
Thanks!
Acked-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCHv2-net-3.8 2/3] qlcnic: set gso_type
@ 2013-02-07 17:08 ` Jitendra Kalsaria
0 siblings, 0 replies; 15+ messages in thread
From: Jitendra Kalsaria @ 2013-02-07 17:08 UTC (permalink / raw)
To: Michael S. Tsirkin, netdev
Cc: Eilon Greenstein, Jeff Kirsher, Jesse Brandeburg, Bruce Allan,
Carolyn Wyborny, Don Skidmore, Greg Rose, Peter P Waskiewicz Jr,
Alex Duyck, John Ronciak, Tushar Dave, Sony Chacko,
Dept-Eng Linux Driver, John Fastabend, David Miller, Jacob Keller,
linux-kernel, e1000-devel@lists.sourceforge.net,
bhutchings@solarflare.com, "eric.dumazet@gma
On 2/7/13 5:13 AM, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>qlcnic set gso_size but not gso type. This leads to crashes
>in macvtap.
>
>Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>---
> drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
>b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
>index 6f82812..09aa310 100644
>--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
>+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
>@@ -986,8 +986,13 @@ qlcnic_process_lro(struct qlcnic_adapter *adapter,
> th->seq = htonl(seq_number);
> length = skb->len;
>
>- if (adapter->flags & QLCNIC_FW_LRO_MSS_CAP)
>+ if (adapter->flags & QLCNIC_FW_LRO_MSS_CAP) {
> skb_shinfo(skb)->gso_size = qlcnic_get_lro_sts_mss(sts_data1);
>+ if (skb->protocol == htons(ETH_P_IPV6))
>+ skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
>+ else
>+ skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
>+ }
>
> if (vid != 0xffff)
> __vlan_hwaccel_put_tag(skb, vid);
>--
>MST
Thanks!
Acked-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCHv2-net-3.8 3/3] bnx2x: set gso_type
2013-02-07 13:12 ` Michael S. Tsirkin
@ 2013-02-07 13:13 ` Michael S. Tsirkin
-1 siblings, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2013-02-07 13:13 UTC (permalink / raw)
To: netdev
Cc: Eilon Greenstein, Jeff Kirsher, Jesse Brandeburg, Bruce Allan,
Carolyn Wyborny, Don Skidmore, Greg Rose, Peter P Waskiewicz Jr,
Alex Duyck, John Ronciak, Tushar Dave, Jitendra Kalsaria,
Sony Chacko, linux-driver, John Fastabend, David S. Miller,
Jacob Keller, linux-kernel, e1000-devel, bhutchings, eric.dumazet
In LRO mode, bnx2x set gso_size but not gso type.
This leads to crashes in macvtap.
Commit cbf1de72324a8105ddcc3d9ce9acbc613faea17e
queued for 3.9 includes a more complete fix.
This is a minimal patch to avoid the crash, for 3.8.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index f771ddf..a5edac8 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -504,13 +504,11 @@ static int bnx2x_fill_frag_skb(struct bnx2x *bp, struct bnx2x_fastpath *fp,
skb_shinfo(skb)->gso_size = bnx2x_set_lro_mss(bp,
tpa_info->parsing_flags, len_on_bd);
- /* set for GRO */
- if (fp->mode == TPA_MODE_GRO)
- skb_shinfo(skb)->gso_type =
- (GET_FLAG(tpa_info->parsing_flags,
- PARSING_FLAGS_OVER_ETHERNET_PROTOCOL) ==
- PRS_FLAG_OVERETH_IPV6) ?
- SKB_GSO_TCPV6 : SKB_GSO_TCPV4;
+ skb_shinfo(skb)->gso_type =
+ (GET_FLAG(tpa_info->parsing_flags,
+ PARSING_FLAGS_OVER_ETHERNET_PROTOCOL) ==
+ PRS_FLAG_OVERETH_IPV6) ?
+ SKB_GSO_TCPV6 : SKB_GSO_TCPV4;
}
--
MST
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCHv2-net-3.8 3/3] bnx2x: set gso_type
@ 2013-02-07 13:13 ` Michael S. Tsirkin
0 siblings, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2013-02-07 13:13 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, e1000-devel, Jitendra Kalsaria, Bruce Allan,
Jesse Brandeburg, Eilon Greenstein, John Fastabend, John Ronciak,
Sony Chacko, linux-driver, bhutchings, linux-kernel, Jacob Keller
In LRO mode, bnx2x set gso_size but not gso type.
This leads to crashes in macvtap.
Commit cbf1de72324a8105ddcc3d9ce9acbc613faea17e
queued for 3.9 includes a more complete fix.
This is a minimal patch to avoid the crash, for 3.8.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index f771ddf..a5edac8 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -504,13 +504,11 @@ static int bnx2x_fill_frag_skb(struct bnx2x *bp, struct bnx2x_fastpath *fp,
skb_shinfo(skb)->gso_size = bnx2x_set_lro_mss(bp,
tpa_info->parsing_flags, len_on_bd);
- /* set for GRO */
- if (fp->mode == TPA_MODE_GRO)
- skb_shinfo(skb)->gso_type =
- (GET_FLAG(tpa_info->parsing_flags,
- PARSING_FLAGS_OVER_ETHERNET_PROTOCOL) ==
- PRS_FLAG_OVERETH_IPV6) ?
- SKB_GSO_TCPV6 : SKB_GSO_TCPV4;
+ skb_shinfo(skb)->gso_type =
+ (GET_FLAG(tpa_info->parsing_flags,
+ PARSING_FLAGS_OVER_ETHERNET_PROTOCOL) ==
+ PRS_FLAG_OVERETH_IPV6) ?
+ SKB_GSO_TCPV6 : SKB_GSO_TCPV4;
}
--
MST
------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCHv2-net-3.8 3/3] bnx2x: set gso_type
2013-02-07 13:13 ` Michael S. Tsirkin
@ 2013-02-07 13:37 ` Dmitry Kravkov
-1 siblings, 0 replies; 15+ messages in thread
From: Dmitry Kravkov @ 2013-02-07 13:37 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: netdev, Eilon Greenstein, Jeff Kirsher, Jesse Brandeburg,
Bruce Allan, Carolyn Wyborny, Don Skidmore, Greg Rose,
Peter P Waskiewicz Jr, Alex Duyck, John Ronciak, Tushar Dave,
Jitendra Kalsaria, Sony Chacko, linux-driver, John Fastabend,
David S. Miller, Jacob Keller, linux-kernel, e1000-devel,
bhutchings, eric.dumazet
On Thu, 2013-02-07 at 15:13 +0200, Michael S. Tsirkin wrote:
> In LRO mode, bnx2x set gso_size but not gso type.
> This leads to crashes in macvtap.
> Commit cbf1de72324a8105ddcc3d9ce9acbc613faea17e
> queued for 3.9 includes a more complete fix.
> This is a minimal patch to avoid the crash, for 3.8.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> index f771ddf..a5edac8 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> @@ -504,13 +504,11 @@ static int bnx2x_fill_frag_skb(struct bnx2x *bp, struct bnx2x_fastpath *fp,
> skb_shinfo(skb)->gso_size = bnx2x_set_lro_mss(bp,
> tpa_info->parsing_flags, len_on_bd);
>
> - /* set for GRO */
> - if (fp->mode == TPA_MODE_GRO)
> - skb_shinfo(skb)->gso_type =
> - (GET_FLAG(tpa_info->parsing_flags,
> - PARSING_FLAGS_OVER_ETHERNET_PROTOCOL) ==
> - PRS_FLAG_OVERETH_IPV6) ?
> - SKB_GSO_TCPV6 : SKB_GSO_TCPV4;
> + skb_shinfo(skb)->gso_type =
> + (GET_FLAG(tpa_info->parsing_flags,
> + PARSING_FLAGS_OVER_ETHERNET_PROTOCOL) ==
> + PRS_FLAG_OVERETH_IPV6) ?
> + SKB_GSO_TCPV6 : SKB_GSO_TCPV4;
> }
>
>
Thanks!
Acked-by: Dmitry Kravkov <dmitry@broadcom.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCHv2-net-3.8 3/3] bnx2x: set gso_type
@ 2013-02-07 13:37 ` Dmitry Kravkov
0 siblings, 0 replies; 15+ messages in thread
From: Dmitry Kravkov @ 2013-02-07 13:37 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Eilon Greenstein, John Fastabend, e1000-devel, Jesse Brandeburg,
Jacob Keller, Sony Chacko, bhutchings, Jitendra Kalsaria, netdev,
Bruce Allan, linux-kernel, John Ronciak, linux-driver,
David S. Miller
On Thu, 2013-02-07 at 15:13 +0200, Michael S. Tsirkin wrote:
> In LRO mode, bnx2x set gso_size but not gso type.
> This leads to crashes in macvtap.
> Commit cbf1de72324a8105ddcc3d9ce9acbc613faea17e
> queued for 3.9 includes a more complete fix.
> This is a minimal patch to avoid the crash, for 3.8.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> index f771ddf..a5edac8 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> @@ -504,13 +504,11 @@ static int bnx2x_fill_frag_skb(struct bnx2x *bp, struct bnx2x_fastpath *fp,
> skb_shinfo(skb)->gso_size = bnx2x_set_lro_mss(bp,
> tpa_info->parsing_flags, len_on_bd);
>
> - /* set for GRO */
> - if (fp->mode == TPA_MODE_GRO)
> - skb_shinfo(skb)->gso_type =
> - (GET_FLAG(tpa_info->parsing_flags,
> - PARSING_FLAGS_OVER_ETHERNET_PROTOCOL) ==
> - PRS_FLAG_OVERETH_IPV6) ?
> - SKB_GSO_TCPV6 : SKB_GSO_TCPV4;
> + skb_shinfo(skb)->gso_type =
> + (GET_FLAG(tpa_info->parsing_flags,
> + PARSING_FLAGS_OVER_ETHERNET_PROTOCOL) ==
> + PRS_FLAG_OVERETH_IPV6) ?
> + SKB_GSO_TCPV6 : SKB_GSO_TCPV4;
> }
>
>
Thanks!
Acked-by: Dmitry Kravkov <dmitry@broadcom.com>
------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCHv2-net-3.8 0/3] fix kernel crash with macvtap on top of LRO
2013-02-07 13:12 ` Michael S. Tsirkin
@ 2013-02-07 17:05 ` Ben Hutchings
-1 siblings, 0 replies; 15+ messages in thread
From: Ben Hutchings @ 2013-02-07 17:05 UTC (permalink / raw)
To: Michael S. Tsirkin, David S. Miller
Cc: netdev, Eilon Greenstein, Jeff Kirsher, Jesse Brandeburg,
Bruce Allan, Carolyn Wyborny, Don Skidmore, Greg Rose,
Peter P Waskiewicz Jr, Alex Duyck, John Ronciak, Tushar Dave,
Jitendra Kalsaria, Sony Chacko, linux-driver, John Fastabend,
Jacob Keller, linux-kernel, e1000-devel, eric.dumazet
On Thu, 2013-02-07 at 15:12 +0200, Michael S. Tsirkin wrote:
> At the moment, macvtap crashes are observed if macvtap is attached
> to an interface with LRO enabled.
> The crash in question is BUG() in macvtap_skb_to_vnet_hdr.
> This happens because several drivers set gso_size but not gso_type
> in incoming skbs.
> This didn't use to be the case: with intel cards on 3.2 and older
> kernels, with qlogic - on 3.4 and older kernels, so it's a regression if
> not a recent one.
> The following patches fix this for qlogic, broadcom and intel drivers.
>
> I tested that the patch fixes the crash for ixgbe but
> don't have qlogic/broadcom hardware to test.
> I also only tested TCPv4.
>
> Please review, and consider for 3.8.
>
> Changes from v1:
> - added missing htons as suggested by Eric
> - backported the relevant bits from
> cbf1de72324a8105ddcc3d9ce9acbc613faea17e for bnx2x
I still don't think macvtap should behave inconsistently with forwarding
and the bridge device. We need a consensus as to whether transformation
by LRO is allowable before making changes one way or the other. If
we're going to allow it then inet_lro also needs this fix.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCHv2-net-3.8 0/3] fix kernel crash with macvtap on top of LRO
@ 2013-02-07 17:05 ` Ben Hutchings
0 siblings, 0 replies; 15+ messages in thread
From: Ben Hutchings @ 2013-02-07 17:05 UTC (permalink / raw)
To: Michael S. Tsirkin, David S. Miller
Cc: Eilon Greenstein, John Fastabend, e1000-devel, Jesse Brandeburg,
Jacob, Keller, Tushar, Sony Chacko, Jitendra Kalsaria, Carolyn,
Jeff, netdev, Bruce Allan, linux-kernel, John Ronciak,
linux-driver, Peter
On Thu, 2013-02-07 at 15:12 +0200, Michael S. Tsirkin wrote:
> At the moment, macvtap crashes are observed if macvtap is attached
> to an interface with LRO enabled.
> The crash in question is BUG() in macvtap_skb_to_vnet_hdr.
> This happens because several drivers set gso_size but not gso_type
> in incoming skbs.
> This didn't use to be the case: with intel cards on 3.2 and older
> kernels, with qlogic - on 3.4 and older kernels, so it's a regression if
> not a recent one.
> The following patches fix this for qlogic, broadcom and intel drivers.
>
> I tested that the patch fixes the crash for ixgbe but
> don't have qlogic/broadcom hardware to test.
> I also only tested TCPv4.
>
> Please review, and consider for 3.8.
>
> Changes from v1:
> - added missing htons as suggested by Eric
> - backported the relevant bits from
> cbf1de72324a8105ddcc3d9ce9acbc613faea17e for bnx2x
I still don't think macvtap should behave inconsistently with forwarding
and the bridge device. We need a consensus as to whether transformation
by LRO is allowable before making changes one way or the other. If
we're going to allow it then inet_lro also needs this fix.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCHv2-net-3.8 0/3] fix kernel crash with macvtap on top of LRO
2013-02-07 13:12 ` Michael S. Tsirkin
` (4 preceding siblings ...)
(?)
@ 2013-02-11 1:15 ` David Miller
-1 siblings, 0 replies; 15+ messages in thread
From: David Miller @ 2013-02-11 1:15 UTC (permalink / raw)
To: mst
Cc: netdev, eilong, jeffrey.t.kirsher, jesse.brandeburg,
bruce.w.allan, carolyn.wyborny, donald.c.skidmore, gregory.v.rose,
peter.p.waskiewicz.jr, alexander.h.duyck, john.ronciak,
tushar.n.dave, jitendra.kalsaria, sony.chacko, linux-driver,
john.r.fastabend, jacob.e.keller, linux-kernel, e1000-devel,
bhutchings, eric.dumazet
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Thu, 7 Feb 2013 15:12:56 +0200
> At the moment, macvtap crashes are observed if macvtap is attached
> to an interface with LRO enabled.
> The crash in question is BUG() in macvtap_skb_to_vnet_hdr.
> This happens because several drivers set gso_size but not gso_type
> in incoming skbs.
> This didn't use to be the case: with intel cards on 3.2 and older
> kernels, with qlogic - on 3.4 and older kernels, so it's a regression if
> not a recent one.
> The following patches fix this for qlogic, broadcom and intel drivers.
>
> I tested that the patch fixes the crash for ixgbe but
> don't have qlogic/broadcom hardware to test.
> I also only tested TCPv4.
>
> Please review, and consider for 3.8.
>
> Changes from v1:
> - added missing htons as suggested by Eric
> - backported the relevant bits from
> cbf1de72324a8105ddcc3d9ce9acbc613faea17e for bnx2x
Applied to 'net', thanks.
^ permalink raw reply [flat|nested] 15+ messages in thread