[net 1/2] ixgbe: Fix issues with SR-IOV loopback when flow control is disabled

[net 1/2] ixgbe: Fix issues with SR-IOV loopback when flow control is disabled

[Jeff Kirsher]

From: Alexander Duyck <alexander.h.duyck@intel.com>

This patch allows us to avoid a Tx hang when SR-IOV is enabled.  This hang
can be triggered by sending small packets at a rate that was triggering Rx
missed errors from the adapter while the internal Tx switch and at least
one VF are enabled.

This was all due to the fact that under heavy stress the Rx FIFO never
drained below the flow control high water mark.  This resulted in the Tx
FIFO being head of line blocked due to the fact that it relies on the flow
control high water mark to determine when it is acceptable for the Tx to
place a packet in the Rx FIFO.

The resolution for this is to set the FCRTH value to the RXPBSIZE - 32 so
that even if the ring is almost completely full we can still place Tx
packets on the Rx ring and drop incoming Rx traffic if we do not have
sufficient space available in the Rx FIFO.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Trested-by: Sibai Li <sibai.li@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_common.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
index 383b941..7d81bee 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
@@ -2011,13 +2011,20 @@ s32 ixgbe_fc_enable_generic(struct ixgbe_hw *hw, s32 packetbuf_num)
 	IXGBE_WRITE_REG(hw, IXGBE_MFLCN, mflcn_reg);
 	IXGBE_WRITE_REG(hw, IXGBE_FCCFG, fccfg_reg);
 
-	fcrth = hw->fc.high_water[packetbuf_num] << 10;
 	fcrtl = hw->fc.low_water << 10;
 
 	if (hw->fc.current_mode & ixgbe_fc_tx_pause) {
+		fcrth = hw->fc.high_water[packetbuf_num] << 10;
 		fcrth |= IXGBE_FCRTH_FCEN;
 		if (hw->fc.send_xon)
 			fcrtl |= IXGBE_FCRTL_XONE;
+	} else {
+		/*
+		 * If Tx flow control is disabled, set our high water mark
+		 * to Rx FIFO size minus 32 in order prevent Tx switch
+		 * loopback from stalling on DMA.
+		 */
+		fcrth = IXGBE_READ_REG(hw, IXGBE_RXPBSIZE(packetbuf_num)) - 32;
 	}
 
 	IXGBE_WRITE_REG(hw, IXGBE_FCRTH_82599(packetbuf_num), fcrth);
-- 
1.7.7.6

[net 2/2] net: fix a bug of dropping FCoE frames when disabling tx ip checksum

[Jeff Kirsher]

From: Yi Zou <yi.zou@intel.com>

Fix a bug when using 'ethtool -K ethx tx off' to turn off tx ip checksum,
FCoE CRC offload should not be impacte. The skb_checksum_help() is needed
only if it's not FCoE traffic for ip checksum, regardless of ethtool toggling
the tx ip checksum on or off.

Signed-off-by: Yi Zou <yi.zou@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 net/core/dev.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 6ca32f6..9e378009 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1913,6 +1913,9 @@ int skb_checksum_help(struct sk_buff *skb)
 	__wsum csum;
 	int ret = 0, offset;
 
+	if (skb->protocol == htons(ETH_P_FCOE))
+		goto out;
+
 	if (skb->ip_summed == CHECKSUM_COMPLETE)
 		goto out_set_summed;
 
-- 
1.7.7.6

Re: [net 2/2] net: fix a bug of dropping FCoE frames when disabling tx ip checksum

[Stephen Hemminger]

On Wed, 14 Mar 2012 00:01:58 -0700
Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:

> From: Yi Zou <yi.zou@intel.com>
> 
> Fix a bug when using 'ethtool -K ethx tx off' to turn off tx ip checksum,
> FCoE CRC offload should not be impacte. The skb_checksum_help() is needed
> only if it's not FCoE traffic for ip checksum, regardless of ethtool toggling
> the tx ip checksum on or off.
> 
> Signed-off-by: Yi Zou <yi.zou@intel.com>
> Tested-by: Ross Brattain <ross.b.brattain@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>  net/core/dev.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 6ca32f6..9e378009 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1913,6 +1913,9 @@ int skb_checksum_help(struct sk_buff *skb)
>  	__wsum csum;
>  	int ret = 0, offset;
>  
> +	if (skb->protocol == htons(ETH_P_FCOE))
> +		goto out;
> +
>  	if (skb->ip_summed == CHECKSUM_COMPLETE)
>  		goto out_set_summed;
>  

Shouldn't this be done in the offending driver rather than the core?

RE: [net 2/2] net: fix a bug of dropping FCoE frames when disabling tx ip checksum

[Zou, Yi]

> 
> On Wed, 14 Mar 2012 00:01:58 -0700
> Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
> 
> > From: Yi Zou <yi.zou@intel.com>
> >
> > Fix a bug when using 'ethtool -K ethx tx off' to turn off tx ip
> checksum,
> > FCoE CRC offload should not be impacte. The skb_checksum_help() is
> needed
> > only if it's not FCoE traffic for ip checksum, regardless of ethtool
> toggling
> > the tx ip checksum on or off.
> >
> > Signed-off-by: Yi Zou <yi.zou@intel.com>
> > Tested-by: Ross Brattain <ross.b.brattain@intel.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > ---
> >  net/core/dev.c |    3 +++
> >  1 files changed, 3 insertions(+), 0 deletions(-)
> >
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 6ca32f6..9e378009 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -1913,6 +1913,9 @@ int skb_checksum_help(struct sk_buff *skb)
> >  	__wsum csum;
> >  	int ret = 0, offset;
> >
> > +	if (skb->protocol == htons(ETH_P_FCOE))
> > +		goto out;
> > +
> >  	if (skb->ip_summed == CHECKSUM_COMPLETE)
> >  		goto out_set_summed;
> >
> 
> Shouldn't this be done in the offending driver rather than the core?
Well, yes and no, the driver is already using NETIF_F_FCOE_CRC to indicate
the FCoE CRC offload capability, and can_checksum_protocol() indicates that.
The fcoe protocol stack then uses CHECKSUM_PARTIAL and NETIF_F_FCOE_CRC to
pass through netdev w/o doing skb_checksum(), which is not captured here
in dev_hard_start_xmit() as it looks at NETIF_F_ALL_CSUM only before calling
skb_checksum_help().

Yi

> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [net 2/2] net: fix a bug of dropping FCoE frames when disabling tx ip checksum

[Stephen Hemminger]

On Wed, 14 Mar 2012 18:54:32 +0000
"Zou, Yi" <yi.zou@intel.com> wrote:

> > 
> > On Wed, 14 Mar 2012 00:01:58 -0700
> > Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
> > 
> > > From: Yi Zou <yi.zou@intel.com>
> > >
> > > Fix a bug when using 'ethtool -K ethx tx off' to turn off tx ip
> > checksum,
> > > FCoE CRC offload should not be impacte. The skb_checksum_help() is
> > needed
> > > only if it's not FCoE traffic for ip checksum, regardless of ethtool
> > toggling
> > > the tx ip checksum on or off.
> > >
> > > Signed-off-by: Yi Zou <yi.zou@intel.com>
> > > Tested-by: Ross Brattain <ross.b.brattain@intel.com>
> > > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > > ---
> > >  net/core/dev.c |    3 +++
> > >  1 files changed, 3 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/net/core/dev.c b/net/core/dev.c
> > > index 6ca32f6..9e378009 100644
> > > --- a/net/core/dev.c
> > > +++ b/net/core/dev.c
> > > @@ -1913,6 +1913,9 @@ int skb_checksum_help(struct sk_buff *skb)
> > >  	__wsum csum;
> > >  	int ret = 0, offset;
> > >
> > > +	if (skb->protocol == htons(ETH_P_FCOE))
> > > +		goto out;
> > > +
> > >  	if (skb->ip_summed == CHECKSUM_COMPLETE)
> > >  		goto out_set_summed;
> > >
> > 
> > Shouldn't this be done in the offending driver rather than the core?
> Well, yes and no, the driver is already using NETIF_F_FCOE_CRC to indicate
> the FCoE CRC offload capability, and can_checksum_protocol() indicates that.
> The fcoe protocol stack then uses CHECKSUM_PARTIAL and NETIF_F_FCOE_CRC to
> pass through netdev w/o doing skb_checksum(), which is not captured here
> in dev_hard_start_xmit() as it looks at NETIF_F_ALL_CSUM only before calling
> skb_checksum_help().

But your change would break any code (if there is any) generating this
kind of packet and going through firewall, etc. For example a bridge or
router for FCOE packets.

Re: [net 2/2] net: fix a bug of dropping FCoE frames when disabling tx ip checksum

[Ben Hutchings]

On Wed, 2012-03-14 at 00:01 -0700, Jeff Kirsher wrote:
> From: Yi Zou <yi.zou@intel.com>
> 
> Fix a bug when using 'ethtool -K ethx tx off' to turn off tx ip checksum,
> FCoE CRC offload should not be impacte. The skb_checksum_help() is needed
> only if it's not FCoE traffic for ip checksum, regardless of ethtool toggling
> the tx ip checksum on or off.
[...]

I think the bug is more fundamental, and it's not just a problem for
FCoE.  For the transmit path, the ip_summed values are specified as:

[forwarding]
 *	COMPLETE: the most generic way. Device supplied checksum of _all_
 *	    the packet as seen by netif_rx in skb->csum.
 *	    NOTE: Even if device supports only some protocols, but
 *	    is able to produce some skb->csum, it MUST use COMPLETE,
 *	    not UNNECESSARY.

[locally-generated]
 *	NONE: skb is checksummed by protocol or csum is not required.
 *
 *	PARTIAL: device is required to csum packet as seen by hard_start_xmit
 *	from skb->csum_start to the end and to record the checksum
 *	at skb->csum_start + skb->csum_offset.

It's implicit that the checksum algorithm for CHECKSUM_PARTIAL is as
specified for TCP/IP.  So none of those is correct when a different
algorithm is to be used.

It seems like we may need another ip_summed value for FCoE, SCTP or any
other protocol with a different checksum algorithm that will be
offloaded.  Maybe allow CHECKSUM_UNNECESSARY to be used on output in
that case?

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.

RE: [net 2/2] net: fix a bug of dropping FCoE frames when disabling tx ip checksum

[Zou, Yi]

> 
> On Wed, 14 Mar 2012 18:54:32 +0000
> "Zou, Yi" <yi.zou@intel.com> wrote:
> 
> > >
> > > On Wed, 14 Mar 2012 00:01:58 -0700
> > > Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
> > >
> > > > From: Yi Zou <yi.zou@intel.com>
> > > >
> > > > Fix a bug when using 'ethtool -K ethx tx off' to turn off tx ip
> > > checksum,
> > > > FCoE CRC offload should not be impacte. The skb_checksum_help() is
> > > needed
> > > > only if it's not FCoE traffic for ip checksum, regardless of
> ethtool
> > > toggling
> > > > the tx ip checksum on or off.
> > > >
> > > > Signed-off-by: Yi Zou <yi.zou@intel.com>
> > > > Tested-by: Ross Brattain <ross.b.brattain@intel.com>
> > > > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > > > ---
> > > >  net/core/dev.c |    3 +++
> > > >  1 files changed, 3 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/net/core/dev.c b/net/core/dev.c
> > > > index 6ca32f6..9e378009 100644
> > > > --- a/net/core/dev.c
> > > > +++ b/net/core/dev.c
> > > > @@ -1913,6 +1913,9 @@ int skb_checksum_help(struct sk_buff *skb)
> > > >  	__wsum csum;
> > > >  	int ret = 0, offset;
> > > >
> > > > +	if (skb->protocol == htons(ETH_P_FCOE))
> > > > +		goto out;
> > > > +
> > > >  	if (skb->ip_summed == CHECKSUM_COMPLETE)
> > > >  		goto out_set_summed;
> > > >
> > >
> > > Shouldn't this be done in the offending driver rather than the core?
> > Well, yes and no, the driver is already using NETIF_F_FCOE_CRC to
> indicate
> > the FCoE CRC offload capability, and can_checksum_protocol() indicates
> that.
> > The fcoe protocol stack then uses CHECKSUM_PARTIAL and NETIF_F_FCOE_CRC
> to
> > pass through netdev w/o doing skb_checksum(), which is not captured
> here
> > in dev_hard_start_xmit() as it looks at NETIF_F_ALL_CSUM only before
> calling
> > skb_checksum_help().
> 
> But your change would break any code (if there is any) generating this
> kind of packet and going through firewall, etc. For example a bridge or
> router for FCOE packets.

I don't know anything like that exists right now but in that case for fcoe,
you dan't want to use CHECKSUM_PARTIAL on output. Still, the point is the 
skb_checksum_help() only makes sense for ip checksum.

yi

RE: [net 2/2] net: fix a bug of dropping FCoE frames when disabling tx ip checksum

[Zou, Yi]

> 
> On Wed, 2012-03-14 at 00:01 -0700, Jeff Kirsher wrote:
> > From: Yi Zou <yi.zou@intel.com>
> >
> > Fix a bug when using 'ethtool -K ethx tx off' to turn off tx ip
> checksum,
> > FCoE CRC offload should not be impacte. The skb_checksum_help() is
> needed
> > only if it's not FCoE traffic for ip checksum, regardless of ethtool
> toggling
> > the tx ip checksum on or off.
> [...]
> 
> I think the bug is more fundamental, and it's not just a problem for
> FCoE.  For the transmit path, the ip_summed values are specified as:
> 
> [forwarding]
>  *	COMPLETE: the most generic way. Device supplied checksum of _all_
>  *	    the packet as seen by netif_rx in skb->csum.
>  *	    NOTE: Even if device supports only some protocols, but
>  *	    is able to produce some skb->csum, it MUST use COMPLETE,
>  *	    not UNNECESSARY.
> 
> [locally-generated]
>  *	NONE: skb is checksummed by protocol or csum is not required.
>  *
>  *	PARTIAL: device is required to csum packet as seen by
> hard_start_xmit
>  *	from skb->csum_start to the end and to record the checksum
>  *	at skb->csum_start + skb->csum_offset.
> 
> It's implicit that the checksum algorithm for CHECKSUM_PARTIAL is as
> specified for TCP/IP.  So none of those is correct when a different
> algorithm is to be used.
> 
> It seems like we may need another ip_summed value for FCoE, SCTP or any
> other protocol with a different checksum algorithm that will be
> offloaded.  Maybe allow CHECKSUM_UNNECESSARY to be used on output in
> that case?
> 
> Ben.

CHECKSUM_UNNECESSARY sounds good to me, if it's ok to be used on the tx path
as well, I think so but I am not 100% sure, it'd resolve this for fcoe and sctp
like, downside is it's a bigger change that requires corresponding changes in
these protocol driver stacks as well.

yi

> 
> --
> 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.

Re: [net 2/2] net: fix a bug of dropping FCoE frames when disabling tx ip checksum

[David Miller]

From: "Zou, Yi" <yi.zou@intel.com>
Date: Wed, 14 Mar 2012 20:48:03 +0000

> CHECKSUM_UNNECESSARY sounds good to me, if it's ok to be used on the tx path
> as well, I think so but I am not 100% sure, it'd resolve this for fcoe and sctp
> like, downside is it's a bigger change that requires corresponding changes in
> these protocol driver stacks as well.

This is definitely the way this bug should be fixed, CHECKSUM_PARTIAL should
never be set for packets outside of the domain that value was designed for.

RE: [net 2/2] net: fix a bug of dropping FCoE frames when disabling tx ip checksum

[Zou, Yi]

> 
> From: "Zou, Yi" <yi.zou@intel.com>
> Date: Wed, 14 Mar 2012 20:48:03 +0000
> 
> > CHECKSUM_UNNECESSARY sounds good to me, if it's ok to be used on the tx
> path 
> > as well, I think so but I am not 100% sure, it'd resolve this for fcoe
> and sctp
> > like, downside is it's a bigger change that requires corresponding
> changes in
> > these protocol driver stacks as well.
> 
> This is definitely the way this bug should be fixed, CHECKSUM_PARTIAL should
> never be set for packets outside of the domain that value was designed for.

Cool, then please drop this patch and I will fix this by changing the fcoe 
protocol stack driver to use CHECKSUM_UNNECESSARY instead.

yi

Re: [net 2/2] net: fix a bug of dropping FCoE frames when disabling tx ip checksum

[David Miller]

From: "Zou, Yi" <yi.zou@intel.com>
Date: Wed, 14 Mar 2012 22:04:06 +0000

>> 
>> From: "Zou, Yi" <yi.zou@intel.com>
>> Date: Wed, 14 Mar 2012 20:48:03 +0000
>> 
>> > CHECKSUM_UNNECESSARY sounds good to me, if it's ok to be used on the tx
>> path 
>> > as well, I think so but I am not 100% sure, it'd resolve this for fcoe
>> and sctp
>> > like, downside is it's a bigger change that requires corresponding
>> changes in
>> > these protocol driver stacks as well.
>> 
>> This is definitely the way this bug should be fixed, CHECKSUM_PARTIAL should
>> never be set for packets outside of the domain that value was designed for.
> 
> Cool, then please drop this patch and I will fix this by changing the fcoe 
> protocol stack driver to use CHECKSUM_UNNECESSARY instead.

I've dropped both patches.

RE: [net 2/2] net: fix a bug of dropping FCoE frames when disabling tx ip checksum

[Ben Hutchings]

On Wed, 2012-03-14 at 20:48 +0000, Zou, Yi wrote:
> > 
> > On Wed, 2012-03-14 at 00:01 -0700, Jeff Kirsher wrote:
> > > From: Yi Zou <yi.zou@intel.com>
> > >
> > > Fix a bug when using 'ethtool -K ethx tx off' to turn off tx ip
> > checksum,
> > > FCoE CRC offload should not be impacte. The skb_checksum_help() is
> > needed
> > > only if it's not FCoE traffic for ip checksum, regardless of ethtool
> > toggling
> > > the tx ip checksum on or off.
> > [...]
> > 
> > I think the bug is more fundamental, and it's not just a problem for
> > FCoE.  For the transmit path, the ip_summed values are specified as:
> > 
> > [forwarding]
> >  *	COMPLETE: the most generic way. Device supplied checksum of _all_
> >  *	    the packet as seen by netif_rx in skb->csum.
> >  *	    NOTE: Even if device supports only some protocols, but
> >  *	    is able to produce some skb->csum, it MUST use COMPLETE,
> >  *	    not UNNECESSARY.
> > 
> > [locally-generated]
> >  *	NONE: skb is checksummed by protocol or csum is not required.
> >  *
> >  *	PARTIAL: device is required to csum packet as seen by
> > hard_start_xmit
> >  *	from skb->csum_start to the end and to record the checksum
> >  *	at skb->csum_start + skb->csum_offset.
> > 
> > It's implicit that the checksum algorithm for CHECKSUM_PARTIAL is as
> > specified for TCP/IP.  So none of those is correct when a different
> > algorithm is to be used.
> > 
> > It seems like we may need another ip_summed value for FCoE, SCTP or any
> > other protocol with a different checksum algorithm that will be
> > offloaded.  Maybe allow CHECKSUM_UNNECESSARY to be used on output in
> > that case?
> > 
> > Ben.
> 
> CHECKSUM_UNNECESSARY sounds good to me, if it's ok to be used on the tx path
> as well,

I don't believe it is yet.

> I think so but I am not 100% sure, it'd resolve this for fcoe and sctp
> like, downside is it's a bigger change that requires corresponding changes in
> these protocol driver stacks as well.

Yes, but this should be done properly rather than patched up with a
bunch of checks for specific protocols.

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.

RE: [net 2/2] net: fix a bug of dropping FCoE frames when disabling tx ip checksum

[Zou, Yi]

> On Wed, 2012-03-14 at 20:48 +0000, Zou, Yi wrote:
> > >
> > > On Wed, 2012-03-14 at 00:01 -0700, Jeff Kirsher wrote:
> > > > From: Yi Zou <yi.zou@intel.com>
> > > >
> > > > Fix a bug when using 'ethtool -K ethx tx off' to turn off tx ip
> > > checksum,
> > > > FCoE CRC offload should not be impacte. The skb_checksum_help() is
> > > needed
> > > > only if it's not FCoE traffic for ip checksum, regardless of
> ethtool
> > > toggling
> > > > the tx ip checksum on or off.
> > > [...]
> > >
> > > I think the bug is more fundamental, and it's not just a problem for
> > > FCoE.  For the transmit path, the ip_summed values are specified as:
> > >
> > > [forwarding]
> > >  *	COMPLETE: the most generic way. Device supplied checksum of
> _all_
> > >  *	    the packet as seen by netif_rx in skb->csum.
> > >  *	    NOTE: Even if device supports only some protocols, but
> > >  *	    is able to produce some skb->csum, it MUST use COMPLETE,
> > >  *	    not UNNECESSARY.
> > >
> > > [locally-generated]
> > >  *	NONE: skb is checksummed by protocol or csum is not required.
> > >  *
> > >  *	PARTIAL: device is required to csum packet as seen by
> > > hard_start_xmit
> > >  *	from skb->csum_start to the end and to record the checksum
> > >  *	at skb->csum_start + skb->csum_offset.
> > >
> > > It's implicit that the checksum algorithm for CHECKSUM_PARTIAL is as
> > > specified for TCP/IP.  So none of those is correct when a different
> > > algorithm is to be used.
> > >
> > > It seems like we may need another ip_summed value for FCoE, SCTP or
> any
> > > other protocol with a different checksum algorithm that will be
> > > offloaded.  Maybe allow CHECKSUM_UNNECESSARY to be used on output in
> > > that case?
> > >
> > > Ben.
> >
> > CHECKSUM_UNNECESSARY sounds good to me, if it's ok to be used on the tx
> path
> > as well,
> 
> I don't believe it is yet.
> 
> > I think so but I am not 100% sure, it'd resolve this for fcoe and sctp
> > like, downside is it's a bigger change that requires corresponding
> changes in
> > these protocol driver stacks as well.
> 
> Yes, but this should be done properly rather than patched up with a
> bunch of checks for specific protocols.
> 
> Ben.
Agreed, however, I will still have to fix the netif_needs_gso() like below
to be able to use CHECKSUM_UNNECESSARY to not go down the dev_gso_segment()
path for FCoE, which is using SKB_GSO_FCOE.

So, anyone see this would break anything? i.e., using CHECKSUM_UNNECESSARY
but still want to do dev_gso_segment? If ok, I will resend the patch to
do the following.

Thanks,
yi

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 0eac07c..c1b2b5f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2636,7 +2636,8 @@ static inline int netif_needs_gso(struct sk_buff *skb,
        netdev_features_t features)
 {
        return skb_is_gso(skb) && (!skb_gso_ok(skb, features) ||
-               unlikely(skb->ip_summed != CHECKSUM_PARTIAL));
+               unlikely((skb->ip_summed != CHECKSUM_PARTIAL) &&
+                        (skb->ip_summed != CHECKSUM_UNNECESSARY)));
 }



> 
> --
> 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.