-----Original Message-----
From: Michael Chan [mailto:michael.chan@broadcom.com]
Sent: Thursday, December 14, 2017 2:16 AM
To: Chopra, Manish <redacted>
Cc: davem@davemloft.net; netdev@vger.kernel.org;
andrew.gospodarek@broadcom.com; Elior, Ariel [off-list ref];
Dept-Eng Everest Linux L2 [off-list ref]
Subject: Re: [PATCH net-next v4 4/5] bnx2x: Use NETIF_F_GRO_HW.

On Wed, Dec 13, 2017 at 1:08 AM, Chopra, Manish
[off-list ref] wrote:

quoted

Hi Michael,  There seems a behavioral change here. This driver support
two HW aggregation modes [LRO and GRO] With the changes, Interfaces

come with HW GRO enabled and LRO disabled by default as opposed to earlier
where interfaces used to come with LRO enabled.

Right.  Before, you had both NETIF_F_GRO and NETIF_F_LRO set and the code
looked at NETIF_F_LRO first and turned on LRO.

Now, we set NETIF_F_GRO and NETIF_F_GRO_HW by default.  NETIF_F_LRO is
turned off since NETIF_F_GRO_HW is on.

If you want, I can change it back to the old default.

Yes please, I think we should not make any default behavior change.
Few more comments -

1). This driver uses module param "disable_tpa" also to completely disable TPA [whether it be LRO or HW GRO] along the initialization flow.

For ex. 

/*Set TPA flags*/
if (bp->disable_tpa) {
        bp->dev->hw_features &= ~NETIF_F_LRO;
        bp->dev->features &= ~NETIF_F_LRO;
}

I think we should also disable HW gro here as well, so that device will come up with no TPA at all.

2). In bnx2x_fix_features() we used to do before these changes -

        /* TPA requires Rx CSUM offloading */
        if (!(features & NETIF_F_RXCSUM)) {
                features &= ~NETIF_F_LRO;
                features &= ~NETIF_F_GRO;
        }

I think we should not turn off SW gro here, we should turn off HW gro here now ?
	

quoted

Also, there seems some problem when turning on LRO even after GRO disable.
When I tried to disable GRO and then tried to enable LRO it didn't go well. Not

sure why ?

I just put an old BCM57810 card into my machine and it works for me.
As long as I turn off GRO or GRO_HW, I can turn on LRO.

[root@localhost bnx2x]# ethtool -K p1p1 lro on Cannot get device udp-
fragmentation-offload settings: Operation not supported Cannot get device
udp-fragmentation-offload settings: Operation not supported Could not change
any device features [root@localhost bnx2x]# ethtool -K p1p1 gro off Cannot get
device udp-fragmentation-offload settings: Operation not supported Cannot get
device udp-fragmentation-offload settings: Operation not supported Actual
changes:
generic-receive-offload: off
large-receive-offload: on
rx-gro-hw: off [requested on]
[root@localhost bnx2x]# ethtool -K p1p1 lro on Cannot get device udp-
fragmentation-offload settings: Operation not supported Cannot get device
udp-fragmentation-offload settings: Operation not supported

Doesn't sound like HW specific.
Probably, I was testing on your older series, I will check again on latest series.