* [PATCH net-next-2.6] IFix IPv6 GSO type checks in Intel ethernet drivers
@ 2010-01-19 21:02 Sridhar Samudrala
2010-01-19 21:11 ` Brandeburg, Jesse
0 siblings, 1 reply; 7+ messages in thread
From: Sridhar Samudrala @ 2010-01-19 21:02 UTC (permalink / raw)
To: Herbert Xu, jeffrey.t.kirsher, jesse.brandeburg; +Cc: netdev
Found this problem when testing IPv6 from a KVM guest to a remote
host via e1000e device on the host.
The following patch fixes the check for IPv6 GSO packet in Intel
ethernet drivers to use skb_is_gso_v6(). SKB_GSO_DODGY is also set
when packets are forwarded from a guest.
Signed-off-by: Sridhar Samudrala <sri@us.ibm.com
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 3d57ca5..2353e95 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -3771,7 +3771,7 @@ static int e1000_tso(struct e1000_adapter *adapter,
0, IPPROTO_TCP, 0);
cmd_length = E1000_TXD_CMD_IP;
ipcse = skb_transport_offset(skb) - 1;
- } else if (skb_shinfo(skb)->gso_type == SKB_GSO_TCPV6) {
+ } else if (skb_is_gso_v6(skb)) {
ipv6_hdr(skb)->payload_len = 0;
tcp_hdr(skb)->check = ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
&ipv6_hdr(skb)->daddr,
diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
index d967949..d4bbdf0 100644
--- a/drivers/net/igb/igb_main.c
+++ b/drivers/net/igb/igb_main.c
@@ -3422,7 +3422,7 @@ static inline int igb_tso_adv(struct igb_ring *tx_ring,
iph->daddr, 0,
IPPROTO_TCP,
0);
- } else if (skb_shinfo(skb)->gso_type == SKB_GSO_TCPV6) {
+ } else if (skb_is_gso_v6(skb)) {
ipv6_hdr(skb)->payload_len = 0;
tcp_hdr(skb)->check = ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
&ipv6_hdr(skb)->daddr,
diff --git a/drivers/net/igbvf/netdev.c b/drivers/net/igbvf/netdev.c
index a6c3920..ce33671 100644
--- a/drivers/net/igbvf/netdev.c
+++ b/drivers/net/igbvf/netdev.c
@@ -1963,7 +1963,7 @@ static int igbvf_tso(struct igbvf_adapter *adapter,
iph->daddr, 0,
IPPROTO_TCP,
0);
- } else if (skb_shinfo(skb)->gso_type == SKB_GSO_TCPV6) {
+ } else if (skb_is_gso_v6(skb)) {
ipv6_hdr(skb)->payload_len = 0;
tcp_hdr(skb)->check = ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
&ipv6_hdr(skb)->daddr,
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index 81971ed..b2c0025 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -5113,7 +5113,7 @@ static int ixgbe_tso(struct ixgbe_adapter *adapter,
iph->daddr, 0,
IPPROTO_TCP,
0);
- } else if (skb_shinfo(skb)->gso_type == SKB_GSO_TCPV6) {
+ } else if (skb_is_gso_v6(skb)) {
ipv6_hdr(skb)->payload_len = 0;
tcp_hdr(skb)->check =
~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
diff --git a/drivers/net/ixgbevf/ixgbevf_main.c b/drivers/net/ixgbevf/ixgbevf_main.c
index 39544af..652b6f4 100644
--- a/drivers/net/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ixgbevf/ixgbevf_main.c
@@ -2756,7 +2756,7 @@ static int ixgbevf_tso(struct ixgbevf_adapter *adapter,
IPPROTO_TCP,
0);
adapter->hw_tso_ctxt++;
- } else if (skb_shinfo(skb)->gso_type == SKB_GSO_TCPV6) {
+ } else if (skb_is_gso_v6(skb)) {
ipv6_hdr(skb)->payload_len = 0;
tcp_hdr(skb)->check =
~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH net-next-2.6] IFix IPv6 GSO type checks in Intel ethernet drivers
2010-01-19 21:02 [PATCH net-next-2.6] IFix IPv6 GSO type checks in Intel ethernet drivers Sridhar Samudrala
@ 2010-01-19 21:11 ` Brandeburg, Jesse
2010-01-19 22:14 ` David Miller
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Brandeburg, Jesse @ 2010-01-19 21:11 UTC (permalink / raw)
To: Sridhar Samudrala; +Cc: Herbert Xu, Kirsher, Jeffrey T, netdev
On Tue, 19 Jan 2010, Sridhar Samudrala wrote:
> Found this problem when testing IPv6 from a KVM guest to a remote
> host via e1000e device on the host.
> The following patch fixes the check for IPv6 GSO packet in Intel
> ethernet drivers to use skb_is_gso_v6(). SKB_GSO_DODGY is also set
> when packets are forwarded from a guest.
>
> Signed-off-by: Sridhar Samudrala <sri@us.ibm.com
Looks fine, thanks! The current code in net-next and 2.6.32 both have
exactly the same condition in skb_is_gso_v6, so I'm not sure that this
patch alone will fix any issues, FYI. If this patch is part of another
set or dependent upon another for the actual change in behavior mentioned
in the comment, I think it should be noted or sent in a series.
Ack-ed by: Jesse Brandeburg <jesse.brandeburg@intel.com>
>
> diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
> index 3d57ca5..2353e95 100644
> --- a/drivers/net/e1000e/netdev.c
> +++ b/drivers/net/e1000e/netdev.c
> @@ -3771,7 +3771,7 @@ static int e1000_tso(struct e1000_adapter *adapter,
> 0, IPPROTO_TCP, 0);
> cmd_length = E1000_TXD_CMD_IP;
> ipcse = skb_transport_offset(skb) - 1;
> - } else if (skb_shinfo(skb)->gso_type == SKB_GSO_TCPV6) {
> + } else if (skb_is_gso_v6(skb)) {
> ipv6_hdr(skb)->payload_len = 0;
> tcp_hdr(skb)->check = ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
> &ipv6_hdr(skb)->daddr,
> diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
> index d967949..d4bbdf0 100644
> --- a/drivers/net/igb/igb_main.c
> +++ b/drivers/net/igb/igb_main.c
> @@ -3422,7 +3422,7 @@ static inline int igb_tso_adv(struct igb_ring *tx_ring,
> iph->daddr, 0,
> IPPROTO_TCP,
> 0);
> - } else if (skb_shinfo(skb)->gso_type == SKB_GSO_TCPV6) {
> + } else if (skb_is_gso_v6(skb)) {
> ipv6_hdr(skb)->payload_len = 0;
> tcp_hdr(skb)->check = ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
> &ipv6_hdr(skb)->daddr,
> diff --git a/drivers/net/igbvf/netdev.c b/drivers/net/igbvf/netdev.c
> index a6c3920..ce33671 100644
> --- a/drivers/net/igbvf/netdev.c
> +++ b/drivers/net/igbvf/netdev.c
> @@ -1963,7 +1963,7 @@ static int igbvf_tso(struct igbvf_adapter *adapter,
> iph->daddr, 0,
> IPPROTO_TCP,
> 0);
> - } else if (skb_shinfo(skb)->gso_type == SKB_GSO_TCPV6) {
> + } else if (skb_is_gso_v6(skb)) {
> ipv6_hdr(skb)->payload_len = 0;
> tcp_hdr(skb)->check = ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
> &ipv6_hdr(skb)->daddr,
> diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
> index 81971ed..b2c0025 100644
> --- a/drivers/net/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ixgbe/ixgbe_main.c
> @@ -5113,7 +5113,7 @@ static int ixgbe_tso(struct ixgbe_adapter *adapter,
> iph->daddr, 0,
> IPPROTO_TCP,
> 0);
> - } else if (skb_shinfo(skb)->gso_type == SKB_GSO_TCPV6) {
> + } else if (skb_is_gso_v6(skb)) {
> ipv6_hdr(skb)->payload_len = 0;
> tcp_hdr(skb)->check =
> ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
> diff --git a/drivers/net/ixgbevf/ixgbevf_main.c b/drivers/net/ixgbevf/ixgbevf_main.c
> index 39544af..652b6f4 100644
> --- a/drivers/net/ixgbevf/ixgbevf_main.c
> +++ b/drivers/net/ixgbevf/ixgbevf_main.c
> @@ -2756,7 +2756,7 @@ static int ixgbevf_tso(struct ixgbevf_adapter *adapter,
> IPPROTO_TCP,
> 0);
> adapter->hw_tso_ctxt++;
> - } else if (skb_shinfo(skb)->gso_type == SKB_GSO_TCPV6) {
> + } else if (skb_is_gso_v6(skb)) {
> ipv6_hdr(skb)->payload_len = 0;
> tcp_hdr(skb)->check =
> ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net-next-2.6] IFix IPv6 GSO type checks in Intel ethernet drivers
2010-01-19 21:11 ` Brandeburg, Jesse
@ 2010-01-19 22:14 ` David Miller
2010-01-19 23:15 ` Jeff Kirsher
2010-01-19 22:36 ` Sridhar Samudrala
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: David Miller @ 2010-01-19 22:14 UTC (permalink / raw)
To: jesse.brandeburg; +Cc: sri, herbert, jeffrey.t.kirsher, netdev
From: "Brandeburg, Jesse" <jesse.brandeburg@intel.com>
Date: Tue, 19 Jan 2010 13:11:00 -0800 (Pacific Standard Time)
>
>
> On Tue, 19 Jan 2010, Sridhar Samudrala wrote:
>
>> Found this problem when testing IPv6 from a KVM guest to a remote
>> host via e1000e device on the host.
>> The following patch fixes the check for IPv6 GSO packet in Intel
>> ethernet drivers to use skb_is_gso_v6(). SKB_GSO_DODGY is also set
>> when packets are forwarded from a guest.
>>
>> Signed-off-by: Sridhar Samudrala <sri@us.ibm.com
>
> Looks fine, thanks! The current code in net-next and 2.6.32 both have
> exactly the same condition in skb_is_gso_v6, so I'm not sure that this
> patch alone will fix any issues, FYI. If this patch is part of another
> set or dependent upon another for the actual change in behavior mentioned
> in the comment, I think it should be noted or sent in a series.
>
> Ack-ed by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Just FYI, I'm assuming I'll get this via Jeff eventually.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net-next-2.6] IFix IPv6 GSO type checks in Intel ethernet drivers
2010-01-19 22:14 ` David Miller
@ 2010-01-19 23:15 ` Jeff Kirsher
0 siblings, 0 replies; 7+ messages in thread
From: Jeff Kirsher @ 2010-01-19 23:15 UTC (permalink / raw)
To: David Miller
Cc: Brandeburg, Jesse, sri@us.ibm.com, herbert@gondor.apana.org.au,
netdev@vger.kernel.org
On Tue, 2010-01-19 at 15:14 -0700, David Miller wrote:
> From: "Brandeburg, Jesse" <jesse.brandeburg@intel.com>
> Date: Tue, 19 Jan 2010 13:11:00 -0800 (Pacific Standard Time)
>
> >
> >
> > On Tue, 19 Jan 2010, Sridhar Samudrala wrote:
> >
> >> Found this problem when testing IPv6 from a KVM guest to a remote
> >> host via e1000e device on the host.
> >> The following patch fixes the check for IPv6 GSO packet in Intel
> >> ethernet drivers to use skb_is_gso_v6(). SKB_GSO_DODGY is also set
> >> when packets are forwarded from a guest.
> >>
> >> Signed-off-by: Sridhar Samudrala <sri@us.ibm.com
> >
> > Looks fine, thanks! The current code in net-next and 2.6.32 both have
> > exactly the same condition in skb_is_gso_v6, so I'm not sure that this
> > patch alone will fix any issues, FYI. If this patch is part of another
> > set or dependent upon another for the actual change in behavior mentioned
> > in the comment, I think it should be noted or sent in a series.
> >
> > Ack-ed by: Jesse Brandeburg <jesse.brandeburg@intel.com>
>
> Just FYI, I'm assuming I'll get this via Jeff eventually.
Correct, I will add this patch to my queue of patches.
Cheers,
Jeff
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next-2.6] IFix IPv6 GSO type checks in Intel ethernet drivers
2010-01-19 21:11 ` Brandeburg, Jesse
2010-01-19 22:14 ` David Miller
@ 2010-01-19 22:36 ` Sridhar Samudrala
2010-01-19 22:39 ` Herbert Xu
2010-01-20 0:44 ` Equal cost multipath for IPv6? Templin, Fred L
3 siblings, 0 replies; 7+ messages in thread
From: Sridhar Samudrala @ 2010-01-19 22:36 UTC (permalink / raw)
To: Brandeburg, Jesse; +Cc: Herbert Xu, Kirsher, Jeffrey T, netdev
On Tue, 2010-01-19 at 13:11 -0800, Brandeburg, Jesse wrote:
>
> On Tue, 19 Jan 2010, Sridhar Samudrala wrote:
>
> > Found this problem when testing IPv6 from a KVM guest to a remote
> > host via e1000e device on the host.
> > The following patch fixes the check for IPv6 GSO packet in Intel
> > ethernet drivers to use skb_is_gso_v6(). SKB_GSO_DODGY is also set
> > when packets are forwarded from a guest.
> >
> > Signed-off-by: Sridhar Samudrala <sri@us.ibm.com
>
> Looks fine, thanks! The current code in net-next and 2.6.32 both have
> exactly the same condition in skb_is_gso_v6, so I'm not sure that this
> patch alone will fix any issues, FYI. If this patch is part of another
> set or dependent upon another for the actual change in behavior mentioned
> in the comment, I think it should be noted or sent in a series.
The bug is in comparing the flag SKB_GSO_TCPV6 directly. I replaced the
compare with the call to skb_is_gso_v6.
This is not a regression and the bug is present even in 2.6.32 and
earlier kernels. So it would be good if this patch can go into 2.6.32
and also -stable kernels.
Thanks
Sridhar
> Ack-ed by: Jesse Brandeburg <jesse.brandeburg@intel.com>
>
> >
> > diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
> > index 3d57ca5..2353e95 100644
> > --- a/drivers/net/e1000e/netdev.c
> > +++ b/drivers/net/e1000e/netdev.c
> > @@ -3771,7 +3771,7 @@ static int e1000_tso(struct e1000_adapter *adapter,
> > 0, IPPROTO_TCP, 0);
> > cmd_length = E1000_TXD_CMD_IP;
> > ipcse = skb_transport_offset(skb) - 1;
> > - } else if (skb_shinfo(skb)->gso_type == SKB_GSO_TCPV6) {
> > + } else if (skb_is_gso_v6(skb)) {
> > ipv6_hdr(skb)->payload_len = 0;
> > tcp_hdr(skb)->check = ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
> > &ipv6_hdr(skb)->daddr,
> > diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
> > index d967949..d4bbdf0 100644
> > --- a/drivers/net/igb/igb_main.c
> > +++ b/drivers/net/igb/igb_main.c
> > @@ -3422,7 +3422,7 @@ static inline int igb_tso_adv(struct igb_ring *tx_ring,
> > iph->daddr, 0,
> > IPPROTO_TCP,
> > 0);
> > - } else if (skb_shinfo(skb)->gso_type == SKB_GSO_TCPV6) {
> > + } else if (skb_is_gso_v6(skb)) {
> > ipv6_hdr(skb)->payload_len = 0;
> > tcp_hdr(skb)->check = ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
> > &ipv6_hdr(skb)->daddr,
> > diff --git a/drivers/net/igbvf/netdev.c b/drivers/net/igbvf/netdev.c
> > index a6c3920..ce33671 100644
> > --- a/drivers/net/igbvf/netdev.c
> > +++ b/drivers/net/igbvf/netdev.c
> > @@ -1963,7 +1963,7 @@ static int igbvf_tso(struct igbvf_adapter *adapter,
> > iph->daddr, 0,
> > IPPROTO_TCP,
> > 0);
> > - } else if (skb_shinfo(skb)->gso_type == SKB_GSO_TCPV6) {
> > + } else if (skb_is_gso_v6(skb)) {
> > ipv6_hdr(skb)->payload_len = 0;
> > tcp_hdr(skb)->check = ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
> > &ipv6_hdr(skb)->daddr,
> > diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
> > index 81971ed..b2c0025 100644
> > --- a/drivers/net/ixgbe/ixgbe_main.c
> > +++ b/drivers/net/ixgbe/ixgbe_main.c
> > @@ -5113,7 +5113,7 @@ static int ixgbe_tso(struct ixgbe_adapter *adapter,
> > iph->daddr, 0,
> > IPPROTO_TCP,
> > 0);
> > - } else if (skb_shinfo(skb)->gso_type == SKB_GSO_TCPV6) {
> > + } else if (skb_is_gso_v6(skb)) {
> > ipv6_hdr(skb)->payload_len = 0;
> > tcp_hdr(skb)->check =
> > ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
> > diff --git a/drivers/net/ixgbevf/ixgbevf_main.c b/drivers/net/ixgbevf/ixgbevf_main.c
> > index 39544af..652b6f4 100644
> > --- a/drivers/net/ixgbevf/ixgbevf_main.c
> > +++ b/drivers/net/ixgbevf/ixgbevf_main.c
> > @@ -2756,7 +2756,7 @@ static int ixgbevf_tso(struct ixgbevf_adapter *adapter,
> > IPPROTO_TCP,
> > 0);
> > adapter->hw_tso_ctxt++;
> > - } else if (skb_shinfo(skb)->gso_type == SKB_GSO_TCPV6) {
> > + } else if (skb_is_gso_v6(skb)) {
> > ipv6_hdr(skb)->payload_len = 0;
> > tcp_hdr(skb)->check =
> > ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
> >
> >
> >
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net-next-2.6] IFix IPv6 GSO type checks in Intel ethernet drivers
2010-01-19 21:11 ` Brandeburg, Jesse
2010-01-19 22:14 ` David Miller
2010-01-19 22:36 ` Sridhar Samudrala
@ 2010-01-19 22:39 ` Herbert Xu
2010-01-20 0:44 ` Equal cost multipath for IPv6? Templin, Fred L
3 siblings, 0 replies; 7+ messages in thread
From: Herbert Xu @ 2010-01-19 22:39 UTC (permalink / raw)
To: Brandeburg, Jesse; +Cc: Sridhar Samudrala, Kirsher, Jeffrey T, netdev
On Tue, Jan 19, 2010 at 01:11:00PM -0800, Brandeburg, Jesse wrote:
>
> Looks fine, thanks! The current code in net-next and 2.6.32 both have
> exactly the same condition in skb_is_gso_v6, so I'm not sure that this
> patch alone will fix any issues, FYI. If this patch is part of another
> set or dependent upon another for the actual change in behavior mentioned
> in the comment, I think it should be noted or sent in a series.
skb_is_gso_v6 does an & test instead of == so the patch is definitely
not a noop.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 7+ messages in thread* Equal cost multipath for IPv6?
2010-01-19 21:11 ` Brandeburg, Jesse
` (2 preceding siblings ...)
2010-01-19 22:39 ` Herbert Xu
@ 2010-01-20 0:44 ` Templin, Fred L
3 siblings, 0 replies; 7+ messages in thread
From: Templin, Fred L @ 2010-01-20 0:44 UTC (permalink / raw)
To: netdev
Is equal cost multipath routing implemented for IPv6?
Are there any plans to implement it?
Thanks - Fred
fred.l.templin@boeing.com
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-01-20 1:02 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-19 21:02 [PATCH net-next-2.6] IFix IPv6 GSO type checks in Intel ethernet drivers Sridhar Samudrala
2010-01-19 21:11 ` Brandeburg, Jesse
2010-01-19 22:14 ` David Miller
2010-01-19 23:15 ` Jeff Kirsher
2010-01-19 22:36 ` Sridhar Samudrala
2010-01-19 22:39 ` Herbert Xu
2010-01-20 0:44 ` Equal cost multipath for IPv6? Templin, Fred L
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).