* [PATCH] bnx2x: fix rx checksum validation for IPv6
@ 2012-09-13 22:59 Michal Schmidt
2012-09-13 23:14 ` Eric Dumazet
0 siblings, 1 reply; 5+ messages in thread
From: Michal Schmidt @ 2012-09-13 22:59 UTC (permalink / raw)
To: netdev
Cc: Eilon Greenstein, Eric Dumazet, Yaniv Rosner, Yuval Mintz,
Merav Sicron, Robert Evans, Tom Herbert, Willem de Bruijn,
David Miller
Commit d6cb3e41 "bnx2x: fix checksum validation" caused a performance
regression for IPv6. Rx checksum offload does not work. IPv6 packets
are passed to the stack with CHECKSUM_NONE.
The hardware obviously cannot perform IP checksum validation for IPv6,
because there is no checksum in the IPv6 header. This should not prevent
us from setting CHECKSUM_UNNECESSARY.
Tested on BCM57711.
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index af20c6e..e8e97a7 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -662,14 +662,16 @@ void bnx2x_csum_validate(struct sk_buff *skb, union eth_rx_cqe *cqe,
struct bnx2x_fastpath *fp,
struct bnx2x_eth_q_stats *qstats)
{
- /* Do nothing if no IP/L4 csum validation was done */
-
+ /* Do nothing if no L4 csum validation was done.
+ * We do not check whether IP csum was validated. For IPv4 we assume
+ * that if the card got as far as validating the L4 csum, it also
+ * validated the IP csum. IPv6 has no IP csum.
+ */
if (cqe->fast_path_cqe.status_flags &
- (ETH_FAST_PATH_RX_CQE_IP_XSUM_NO_VALIDATION_FLG |
- ETH_FAST_PATH_RX_CQE_L4_XSUM_NO_VALIDATION_FLG))
+ ETH_FAST_PATH_RX_CQE_L4_XSUM_NO_VALIDATION_FLG)
return;
- /* If both IP/L4 validation were done, check if an error was found. */
+ /* If L4 validation was done, check if an error was found. */
if (cqe->fast_path_cqe.type_error_flags &
(ETH_FAST_PATH_RX_CQE_IP_BAD_XSUM_FLG |
--
1.7.11.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] bnx2x: fix rx checksum validation for IPv6
2012-09-13 22:59 [PATCH] bnx2x: fix rx checksum validation for IPv6 Michal Schmidt
@ 2012-09-13 23:14 ` Eric Dumazet
2012-09-14 5:20 ` Eilon Greenstein
2012-09-18 20:17 ` David Miller
0 siblings, 2 replies; 5+ messages in thread
From: Eric Dumazet @ 2012-09-13 23:14 UTC (permalink / raw)
To: Michal Schmidt
Cc: netdev, Eilon Greenstein, Eric Dumazet, Yaniv Rosner, Yuval Mintz,
Merav Sicron, Robert Evans, Tom Herbert, Willem de Bruijn,
David Miller, Havard Skinnemoen
On Fri, 2012-09-14 at 00:59 +0200, Michal Schmidt wrote:
> Commit d6cb3e41 "bnx2x: fix checksum validation" caused a performance
> regression for IPv6. Rx checksum offload does not work. IPv6 packets
> are passed to the stack with CHECKSUM_NONE.
>
> The hardware obviously cannot perform IP checksum validation for IPv6,
> because there is no checksum in the IPv6 header. This should not prevent
> us from setting CHECKSUM_UNNECESSARY.
>
> Tested on BCM57711.
>
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
> ---
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> index af20c6e..e8e97a7 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> @@ -662,14 +662,16 @@ void bnx2x_csum_validate(struct sk_buff *skb, union eth_rx_cqe *cqe,
> struct bnx2x_fastpath *fp,
> struct bnx2x_eth_q_stats *qstats)
> {
> - /* Do nothing if no IP/L4 csum validation was done */
> -
> + /* Do nothing if no L4 csum validation was done.
> + * We do not check whether IP csum was validated. For IPv4 we assume
> + * that if the card got as far as validating the L4 csum, it also
> + * validated the IP csum. IPv6 has no IP csum.
> + */
> if (cqe->fast_path_cqe.status_flags &
> - (ETH_FAST_PATH_RX_CQE_IP_XSUM_NO_VALIDATION_FLG |
> - ETH_FAST_PATH_RX_CQE_L4_XSUM_NO_VALIDATION_FLG))
> + ETH_FAST_PATH_RX_CQE_L4_XSUM_NO_VALIDATION_FLG)
> return;
>
> - /* If both IP/L4 validation were done, check if an error was found. */
> + /* If L4 validation was done, check if an error was found. */
>
> if (cqe->fast_path_cqe.type_error_flags &
> (ETH_FAST_PATH_RX_CQE_IP_BAD_XSUM_FLG |
Thanks for fixing this bug !
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] bnx2x: fix rx checksum validation for IPv6
2012-09-13 23:14 ` Eric Dumazet
@ 2012-09-14 5:20 ` Eilon Greenstein
2012-09-14 17:39 ` Eilon Greenstein
2012-09-18 20:17 ` David Miller
1 sibling, 1 reply; 5+ messages in thread
From: Eilon Greenstein @ 2012-09-14 5:20 UTC (permalink / raw)
To: Michal Schmidt
Cc: Eric Dumazet, netdev, Eric Dumazet, Yaniv Rosner, Yuval Mintz,
Merav Sicron, Robert Evans, Tom Herbert, Willem de Bruijn,
David Miller, Havard Skinnemoen
On Fri, 2012-09-14 at 01:14 +0200, Eric Dumazet wrote:
> On Fri, 2012-09-14 at 00:59 +0200, Michal Schmidt wrote:
> > Commit d6cb3e41 "bnx2x: fix checksum validation" caused a performance
> > regression for IPv6. Rx checksum offload does not work. IPv6 packets
> > are passed to the stack with CHECKSUM_NONE.
> >
> > The hardware obviously cannot perform IP checksum validation for IPv6,
> > because there is no checksum in the IPv6 header. This should not prevent
> > us from setting CHECKSUM_UNNECESSARY.
> >
> > Tested on BCM57711.
> >
> > Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
> > ---
> > drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 12 +++++++-----
> > 1 file changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> > index af20c6e..e8e97a7 100644
> > --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> > +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> > @@ -662,14 +662,16 @@ void bnx2x_csum_validate(struct sk_buff *skb, union eth_rx_cqe *cqe,
> > struct bnx2x_fastpath *fp,
> > struct bnx2x_eth_q_stats *qstats)
> > {
> > - /* Do nothing if no IP/L4 csum validation was done */
> > -
> > + /* Do nothing if no L4 csum validation was done.
> > + * We do not check whether IP csum was validated. For IPv4 we assume
> > + * that if the card got as far as validating the L4 csum, it also
> > + * validated the IP csum. IPv6 has no IP csum.
> > + */
> > if (cqe->fast_path_cqe.status_flags &
> > - (ETH_FAST_PATH_RX_CQE_IP_XSUM_NO_VALIDATION_FLG |
> > - ETH_FAST_PATH_RX_CQE_L4_XSUM_NO_VALIDATION_FLG))
> > + ETH_FAST_PATH_RX_CQE_L4_XSUM_NO_VALIDATION_FLG)
> > return;
> >
> > - /* If both IP/L4 validation were done, check if an error was found. */
> > + /* If L4 validation was done, check if an error was found. */
> >
> > if (cqe->fast_path_cqe.type_error_flags &
> > (ETH_FAST_PATH_RX_CQE_IP_BAD_XSUM_FLG |
>
> Thanks for fixing this bug !
>
> Acked-by: Eric Dumazet <edumazet@google.com>
Indeed - thanks Michal!
Acked-by: Eilon Greenstein <eilong@broadcom.com>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] bnx2x: fix rx checksum validation for IPv6
2012-09-14 5:20 ` Eilon Greenstein
@ 2012-09-14 17:39 ` Eilon Greenstein
0 siblings, 0 replies; 5+ messages in thread
From: Eilon Greenstein @ 2012-09-14 17:39 UTC (permalink / raw)
To: Michal Schmidt, Eric Dumazet
Cc: netdev, Eric Dumazet, Yaniv Rosner, Yuval Mintz, Merav Sicron,
Robert Evans, Tom Herbert, Willem de Bruijn, David Miller,
Havard Skinnemoen
On Fri, 2012-09-14 at 08:20 +0300, Eilon Greenstein wrote:
> On Fri, 2012-09-14 at 01:14 +0200, Eric Dumazet wrote:
> > On Fri, 2012-09-14 at 00:59 +0200, Michal Schmidt wrote:
> > > Commit d6cb3e41 "bnx2x: fix checksum validation" caused a performance
> > > regression for IPv6. Rx checksum offload does not work. IPv6 packets
> > > are passed to the stack with CHECKSUM_NONE.
> > >
> > > The hardware obviously cannot perform IP checksum validation for IPv6,
> > > because there is no checksum in the IPv6 header. This should not prevent
> > > us from setting CHECKSUM_UNNECESSARY.
> > >
> > > Tested on BCM57711.
> > >
> > > Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
> > > ---
> > > drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 12 +++++++-----
> > > 1 file changed, 7 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> > > index af20c6e..e8e97a7 100644
> > > --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> > > +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> > > @@ -662,14 +662,16 @@ void bnx2x_csum_validate(struct sk_buff *skb, union eth_rx_cqe *cqe,
> > > struct bnx2x_fastpath *fp,
> > > struct bnx2x_eth_q_stats *qstats)
> > > {
> > > - /* Do nothing if no IP/L4 csum validation was done */
> > > -
> > > + /* Do nothing if no L4 csum validation was done.
> > > + * We do not check whether IP csum was validated. For IPv4 we assume
> > > + * that if the card got as far as validating the L4 csum, it also
> > > + * validated the IP csum. IPv6 has no IP csum.
> > > + */
> > > if (cqe->fast_path_cqe.status_flags &
> > > - (ETH_FAST_PATH_RX_CQE_IP_XSUM_NO_VALIDATION_FLG |
> > > - ETH_FAST_PATH_RX_CQE_L4_XSUM_NO_VALIDATION_FLG))
> > > + ETH_FAST_PATH_RX_CQE_L4_XSUM_NO_VALIDATION_FLG)
> > > return;
> > >
> > > - /* If both IP/L4 validation were done, check if an error was found. */
> > > + /* If L4 validation was done, check if an error was found. */
> > >
> > > if (cqe->fast_path_cqe.type_error_flags &
> > > (ETH_FAST_PATH_RX_CQE_IP_BAD_XSUM_FLG |
> >
> > Thanks for fixing this bug !
> >
> > Acked-by: Eric Dumazet <edumazet@google.com>
>
> Indeed - thanks Michal!
>
> Acked-by: Eilon Greenstein <eilong@broadcom.com>
>
Just in case someone will look up this thread in the future, I’m adding
some details: the comment is almost right, the HW does not verify the
IPv6 header in case it contains extension headers this is why this
patch is required (though some users with IPv6 that does not use any
extension headers will not see the issue).
In other words: Eric this is how we both missed it…
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] bnx2x: fix rx checksum validation for IPv6
2012-09-13 23:14 ` Eric Dumazet
2012-09-14 5:20 ` Eilon Greenstein
@ 2012-09-18 20:17 ` David Miller
1 sibling, 0 replies; 5+ messages in thread
From: David Miller @ 2012-09-18 20:17 UTC (permalink / raw)
To: eric.dumazet
Cc: mschmidt, netdev, eilong, edumazet, yanivr, yuvalmin, meravs,
evansr, therbert, willemb, hskinnemoen
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 14 Sep 2012 01:14:39 +0200
> On Fri, 2012-09-14 at 00:59 +0200, Michal Schmidt wrote:
>> Commit d6cb3e41 "bnx2x: fix checksum validation" caused a performance
>> regression for IPv6. Rx checksum offload does not work. IPv6 packets
>> are passed to the stack with CHECKSUM_NONE.
>>
>> The hardware obviously cannot perform IP checksum validation for IPv6,
>> because there is no checksum in the IPv6 header. This should not prevent
>> us from setting CHECKSUM_UNNECESSARY.
>>
>> Tested on BCM57711.
>>
>> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
...
> Thanks for fixing this bug !
>
> Acked-by: Eric Dumazet <edumazet@google.com>
>
>
Applied, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-09-18 20:17 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-13 22:59 [PATCH] bnx2x: fix rx checksum validation for IPv6 Michal Schmidt
2012-09-13 23:14 ` Eric Dumazet
2012-09-14 5:20 ` Eilon Greenstein
2012-09-14 17:39 ` Eilon Greenstein
2012-09-18 20:17 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox