* [PATCH V1 net-next 1/1] hyperv: Properly handle checksum offload
@ 2014-04-26 3:55 K. Y. Srinivasan
2014-04-28 17:23 ` David Miller
0 siblings, 1 reply; 3+ messages in thread
From: K. Y. Srinivasan @ 2014-04-26 3:55 UTC (permalink / raw
To: davem, netdev, linux-kernel, devel, olaf, apw, jasowang; +Cc: stable
Do checksum offload only if the client of the driver wants checksum to be
offloaded.
In this version of the patch, I have addressed comments from
Stephen Hemminger <stephen@networkplumber.org> and
Eric Dumazet <eric.dumazet@gmail.com>.
This patch fixes a bug that is exposed in gateway scenarios.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
Cc: stable@kernel.org
---
drivers/net/hyperv/netvsc_drv.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 939e3af..7ca9daa 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -467,6 +467,9 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
if (skb_is_gso(skb))
goto do_lso;
+ if (skb->ip_summed != CHECKSUM_PARTIAL)
+ goto do_send;
+
rndis_msg_size += NDIS_CSUM_PPI_SIZE;
ppi = init_ppi_data(rndis_msg, NDIS_CSUM_PPI_SIZE,
TCPIP_CHKSUM_PKTINFO);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH V1 net-next 1/1] hyperv: Properly handle checksum offload
2014-04-26 3:55 [PATCH V1 net-next 1/1] hyperv: Properly handle checksum offload K. Y. Srinivasan
@ 2014-04-28 17:23 ` David Miller
2014-04-28 18:31 ` KY Srinivasan
0 siblings, 1 reply; 3+ messages in thread
From: David Miller @ 2014-04-28 17:23 UTC (permalink / raw
To: kys; +Cc: olaf, netdev, jasowang, linux-kernel, apw, devel, stable
From: "K. Y. Srinivasan" <kys@microsoft.com>
Date: Fri, 25 Apr 2014 20:55:23 -0700
> @@ -467,6 +467,9 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
> if (skb_is_gso(skb))
> goto do_lso;
>
> + if (skb->ip_summed != CHECKSUM_PARTIAL)
> + goto do_send;
> +
There are many possible values of ip_summed, why would you go to do_send if
for example it was set to CHECKSUM_COMPLETE?
I think you are just rushing this change, take your time and implement the
fix properly.
For example, if you only expect two possible values here (CHECKSUM_PARTIAL
and something else), design your test so that it only allows those two
values and therefore you'll be documenting this invariant.
^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [PATCH V1 net-next 1/1] hyperv: Properly handle checksum offload
2014-04-28 17:23 ` David Miller
@ 2014-04-28 18:31 ` KY Srinivasan
0 siblings, 0 replies; 3+ messages in thread
From: KY Srinivasan @ 2014-04-28 18:31 UTC (permalink / raw
To: David Miller
Cc: olaf@aepfle.de, netdev@vger.kernel.org, jasowang@redhat.com,
linux-kernel@vger.kernel.org, apw@canonical.com,
devel@linuxdriverproject.org, stable@kernel.org
> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Monday, April 28, 2014 10:24 AM
> To: KY Srinivasan
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com; stable@kernel.org
> Subject: Re: [PATCH V1 net-next 1/1] hyperv: Properly handle checksum
> offload
>
> From: "K. Y. Srinivasan" <kys@microsoft.com>
> Date: Fri, 25 Apr 2014 20:55:23 -0700
>
> > @@ -467,6 +467,9 @@ static int netvsc_start_xmit(struct sk_buff *skb,
> struct net_device *net)
> > if (skb_is_gso(skb))
> > goto do_lso;
> >
> > + if (skb->ip_summed != CHECKSUM_PARTIAL)
> > + goto do_send;
> > +
>
> There are many possible values of ip_summed, why would you go to
> do_send if for example it was set to CHECKSUM_COMPLETE?
>
> I think you are just rushing this change, take your time and implement the fix
> properly.
>
> For example, if you only expect two possible values here
> (CHECKSUM_PARTIAL and something else), design your test so that it only
> allows those two values and therefore you'll be documenting this invariant.
David,
I based this check on the check I see in tcp_v4_send_check() (and tcp_v6_send_check).
Looking at the code some more, it looks like if ip_summed is set to CHECKSUM_NONE, the
software has computed the checksum and we can bypass the code for offloading the checksum.
Regards,
K. Y
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-04-28 18:31 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-26 3:55 [PATCH V1 net-next 1/1] hyperv: Properly handle checksum offload K. Y. Srinivasan
2014-04-28 17:23 ` David Miller
2014-04-28 18:31 ` KY Srinivasan
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 read-only IMAP folder(s) and NNTP newsgroup(s).