netdev - RE: [PATCH net-next v4 4/5] bnx2x: Use NETIF_F_GRO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 13 Dec 2017 09:08:02 +0000
From:   "Chopra, Manish" <Manish.Chopra@...ium.com>
To:     Michael Chan <michael.chan@...adcom.com>,
        "davem@...emloft.net" <davem@...emloft.net>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "andrew.gospodarek@...adcom.com" <andrew.gospodarek@...adcom.com>,
        "Elior, Ariel" <Ariel.Elior@...ium.com>,
        Dept-Eng Everest Linux L2 <Dept-EngEverestLinuxL2@...ium.com>
Subject: RE: [PATCH net-next v4 4/5] bnx2x: Use NETIF_F_GRO_HW.

> -----Original Message-----
> From: Michael Chan [mailto:michael.chan@...adcom.com]
> Sent: Monday, December 11, 2017 5:11 PM
> To: davem@...emloft.net
> Cc: netdev@...r.kernel.org; andrew.gospodarek@...adcom.com; Elior, Ariel
> <Ariel.Elior@...ium.com>; Dept-Eng Everest Linux L2 <Dept-
> EngEverestLinuxL2@...ium.com>
> Subject: [PATCH net-next v4 4/5] bnx2x: Use NETIF_F_GRO_HW.
> 
> Advertise NETIF_F_GRO_HW and turn on TPA_MODE_GRO when
> NETIF_F_GRO_HW is set.  Disable NETIF_F_GRO_HW in bnx2x_fix_features() if
> the MTU does not support TPA_MODE_GRO or GRO is not set.
> bnx2x_change_mtu() also needs to disable NETIF_F_GRO_HW if the MTU does
> not support it.
> 
> Cc: Ariel Elior <Ariel.Elior@...ium.com>
> Cc: everest-linux-l2@...ium.com
> Signed-off-by: Michael Chan <michael.chan@...adcom.com>
> ---
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c  | 19 ++++++++++++-------
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c |  4 +++-
>  2 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> index 4c739d5..f9b18a7 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> @@ -2482,8 +2482,7 @@ static void bnx2x_bz_fp(struct bnx2x *bp, int index)
>  	 */
>  	if (bp->dev->features & NETIF_F_LRO)
>  		fp->mode = TPA_MODE_LRO;
> -	else if (bp->dev->features & NETIF_F_GRO &&
> -		 bnx2x_mtu_allows_gro(bp->dev->mtu))
> +	else if (bp->dev->features & NETIF_F_GRO_HW)
>  		fp->mode = TPA_MODE_GRO;
>  	else
>  		fp->mode = TPA_MODE_DISABLED;
> @@ -4874,6 +4873,9 @@ int bnx2x_change_mtu(struct net_device *dev, int
> new_mtu)
>  	 */
>  	dev->mtu = new_mtu;
> 
> +	if (!bnx2x_mtu_allows_gro(new_mtu))
> +		dev->features &= ~NETIF_F_GRO_HW;
> +
>  	if (IS_PF(bp) && SHMEM2_HAS(bp, curr_cfg))
>  		SHMEM2_WR(bp, curr_cfg, CURR_CFG_MET_OS);
> 
> @@ -4907,6 +4909,10 @@ netdev_features_t bnx2x_fix_features(struct
> net_device *dev,
>  		features &= ~NETIF_F_LRO;
>  		features &= ~NETIF_F_GRO;
>  	}
> +	if (!(features & NETIF_F_GRO) || !bnx2x_mtu_allows_gro(dev->mtu))
> +		features &= ~NETIF_F_GRO_HW;
> +	if (features & NETIF_F_GRO_HW)
> +		features &= ~NETIF_F_LRO;
> 
>  	return features;
>  }
> @@ -4933,13 +4939,12 @@ int bnx2x_set_features(struct net_device *dev,
> netdev_features_t features)
>  		}
>  	}
> 
> -	/* if GRO is changed while LRO is enabled, don't force a reload */
> -	if ((changes & NETIF_F_GRO) && (features & NETIF_F_LRO))
> -		changes &= ~NETIF_F_GRO;
> +	/* Don't care about GRO changes */
> +	changes &= ~NETIF_F_GRO;
> 
>  	/* if GRO is changed while HW TPA is off, don't force a reload */
> -	if ((changes & NETIF_F_GRO) && bp->disable_tpa)
> -		changes &= ~NETIF_F_GRO;
> +	if ((changes & NETIF_F_GRO_HW) && bp->disable_tpa)
> +		changes &= ~NETIF_F_GRO_HW;
> 
>  	if (changes)
>  		bnx2x_reload = true;
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> index 91e2a75..1c7f821 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> @@ -13273,7 +13273,7 @@ static int bnx2x_init_dev(struct bnx2x *bp, struct
> pci_dev *pdev,
> 
>  	dev->hw_features = NETIF_F_SG | NETIF_F_IP_CSUM |
> NETIF_F_IPV6_CSUM |
>  		NETIF_F_TSO | NETIF_F_TSO_ECN | NETIF_F_TSO6 |
> -		NETIF_F_RXCSUM | NETIF_F_LRO | NETIF_F_GRO |
> +		NETIF_F_RXCSUM | NETIF_F_LRO | NETIF_F_GRO |
> NETIF_F_GRO_HW |
>  		NETIF_F_RXHASH | NETIF_F_HW_VLAN_CTAG_TX;
>  	if (!chip_is_e1x) {
>  		dev->hw_features |= NETIF_F_GSO_GRE |
> NETIF_F_GSO_GRE_CSUM | @@ -13309,6 +13309,8 @@ static int
> bnx2x_init_dev(struct bnx2x *bp, struct pci_dev *pdev,
> 
>  	dev->features |= dev->hw_features | NETIF_F_HW_VLAN_CTAG_RX;
>  	dev->features |= NETIF_F_HIGHDMA;
> +	if (dev->features & NETIF_F_GRO_HW)
> +		dev->features &= ~NETIF_F_LRO;
> 
>  	/* Add Loopback capability to the device */
>  	dev->hw_features |= NETIF_F_LOOPBACK;
> --
> 1.8.3.1

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.

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 ?

ethtool -K p3p1 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
rx-gro-hw: off [requested on]

ethtool -K p3p1 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

Thanks,
Manish