[PATCH net-next v4 0/5] Introduce NETIF_F_GRO_HW

Introduce NETIF_F_GRO_HW feature flag and convert drivers that support
hardware GRO to use the new flag.

v4:
- more changes requested by Alexander Duyck:
- check GRO_HW/GRO dependency in drivers's ndo_fix_features().
- Reverse the order of RXCSUM and GRO_HW dependency check in
netdev_fix_features().
- No propagation in netdev_disable_gro_hw().

v3:
- Let driver's ndo_fix_features() disable NETIF_F_LRO when NETIF_F_GRO_HW
is set instead of doing it in common netdev_fix_features().

v2:
- NETIF_F_GRO_HW flag propagation between upper and lower devices not
required (see patch 1).
- NETIF_F_GRO_HW depends on NETIF_F_GRO and NETIF_F_RXCSUM.
- Add dev_disable_gro_hw() to disable GRO_HW for generic XDP.
- Use ndo_fix_features() on all 3 drivers to drop GRO_HW when it is not
supported

Michael Chan (5):
  net: Introduce NETIF_F_GRO_HW.
  net: Disable GRO_HW when generic XDP is installed on a device.
  bnxt_en: Use NETIF_F_GRO_HW.
  bnx2x: Use NETIF_F_GRO_HW.
  qede: Use NETIF_F_GRO_HW.

 Documentation/networking/netdev-features.txt     |  8 +++++++
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c  | 19 ++++++++++------
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c |  4 +++-
 drivers/net/ethernet/broadcom/bnxt/bnxt.c        | 27 +++++++++++++++++------
 drivers/net/ethernet/qlogic/qede/qede.h          |  2 ++
 drivers/net/ethernet/qlogic/qede/qede_ethtool.c  |  3 +++
 drivers/net/ethernet/qlogic/qede/qede_filter.c   | 21 ++++++++++++------
 drivers/net/ethernet/qlogic/qede/qede_main.c     | 17 +++++---------
 include/linux/netdev_features.h                  |  3 +++
 net/core/dev.c                                   | 28 ++++++++++++++++++++++++
 net/core/ethtool.c                               |  1 +
 11 files changed, 100 insertions(+), 33 deletions(-)

-- 
1.8.3.1

Introduce NETIF_F_GRO_HW feature flag for NICs that support hardware
GRO.  With this flag, we can now independently turn on or off hardware
GRO when GRO is on.  Previously, drivers were using NETIF_F_GRO to
control hardware GRO and so it cannot be independently turned on or
off without affecting GRO.

Hardware GRO (just like GRO) guarantees that packets can be re-segmented
by TSO/GSO to reconstruct the original packet stream.  Logically,
GRO_HW should depend on GRO since it a subset, but we will let
individual drivers enforce this dependency as they see fit.

Since NETIF_F_GRO is not propagated between upper and lower devices,
NETIF_F_GRO_HW should follow suit since it is a subset of GRO.  In other
words, a lower device can independent have GRO/GRO_HW enabled or disabled
and no feature propagation is required.  This will preserve the current
GRO behavior.  This can be changed later if we decide to propagate GRO/
GRO_HW/RXCSUM from upper to lower devices.

Cc: Ariel Elior <redacted>
Cc: everest-linux-l2@cavium.com
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 Documentation/networking/netdev-features.txt |  8 ++++++++
 include/linux/netdev_features.h              |  3 +++
 net/core/dev.c                               | 10 ++++++++++
 net/core/ethtool.c                           |  1 +
 4 files changed, 22 insertions(+)

diff --git a/Documentation/networking/netdev-features.txt b/Documentation/networking/netdev-features.txt
index 7413eb0..8f36527 100644
--- a/Documentation/networking/netdev-features.txt
+++ b/Documentation/networking/netdev-features.txt

@@ -163,3 +163,11 @@ This requests that the NIC receive all possible frames, including errored
 frames (such as bad FCS, etc).  This can be helpful when sniffing a link with
 bad packets on it.  Some NICs may receive more packets if also put into normal
 PROMISC mode.
+
+*  rx-gro-hw
+
+This requests that the NIC enables Hardware GRO (generic receive offload).
+Hardware GRO is basically the exact reverse of TSO, and is generally
+stricter than Hardware LRO.  A packet stream merged by Hardware GRO must
+be re-segmentable by GSO or TSO back to the exact original packet stream.
+Hardware GRO is dependent on GRO and RXCSUM.

diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
index b1b0ca7..db84c51 100644
--- a/include/linux/netdev_features.h
+++ b/include/linux/netdev_features.h

@@ -78,6 +78,8 @@ enum {
 	NETIF_F_HW_ESP_TX_CSUM_BIT,	/* ESP with TX checksum offload */
 	NETIF_F_RX_UDP_TUNNEL_PORT_BIT, /* Offload of RX port for UDP tunnels */
 
+	NETIF_F_GRO_HW_BIT,		/* Hardware Generic receive offload */
+
 	/*
 	 * Add your fresh new feature above and remember to update
 	 * netdev_features_strings[] in net/core/ethtool.c and maybe

@@ -97,6 +99,7 @@ enum {
 #define NETIF_F_FRAGLIST	__NETIF_F(FRAGLIST)
 #define NETIF_F_FSO		__NETIF_F(FSO)
 #define NETIF_F_GRO		__NETIF_F(GRO)
+#define NETIF_F_GRO_HW		__NETIF_F(GRO_HW)
 #define NETIF_F_GSO		__NETIF_F(GSO)
 #define NETIF_F_GSO_ROBUST	__NETIF_F(GSO_ROBUST)
 #define NETIF_F_HIGHDMA		__NETIF_F(HIGHDMA)

diff --git a/net/core/dev.c b/net/core/dev.c
index 8aa2f70..bf0149e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c

@@ -7424,6 +7424,16 @@ static netdev_features_t netdev_fix_features(struct net_device *dev,
 		features &= ~dev->gso_partial_features;
 	}
 
+	if (!(features & NETIF_F_RXCSUM)) {
+		/* NETIF_F_GRO_HW implies doing RXCSUM.  If the user does not
+		 * want to enable RXCSUM, logically, we should disable GRO_HW.
+		 */
+		if (features & NETIF_F_GRO_HW) {
+			netdev_dbg(dev, "Dropping NETIF_F_GRO_HW since no RXCSUM feature.\n");
+			features &= ~NETIF_F_GRO_HW;
+		}
+	}
+
 	return features;
 }
 

diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index f8fcf45..50a7920 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c

@@ -73,6 +73,7 @@ int ethtool_op_get_ts_info(struct net_device *dev, struct ethtool_ts_info *info)
 	[NETIF_F_LLTX_BIT] =             "tx-lockless",
 	[NETIF_F_NETNS_LOCAL_BIT] =      "netns-local",
 	[NETIF_F_GRO_BIT] =              "rx-gro",
+	[NETIF_F_GRO_HW_BIT] =           "rx-gro-hw",
 	[NETIF_F_LRO_BIT] =              "rx-lro",
 
 	[NETIF_F_TSO_BIT] =              "tx-tcp-segmentation",

-- 
1.8.3.1

Hardware should not aggregate any packets when generic XDP is installed.

Cc: Ariel Elior <redacted>
Cc: everest-linux-l2@cavium.com
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 net/core/dev.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/net/core/dev.c b/net/core/dev.c
index bf0149e..5e47779 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c

@@ -1542,6 +1542,23 @@ void dev_disable_lro(struct net_device *dev)
 }
 EXPORT_SYMBOL(dev_disable_lro);
 
+/**
+ *	dev_disable_gro_hw - disable HW Generic Receive Offload on a device
+ *	@dev: device
+ *
+ *	Disable HW Generic Receive Offload (GRO_HW) on a net device.  Must be
+ *	called under RTNL.  This is needed if Generic XDP is installed on
+ *	the device.
+ */
+static void dev_disable_gro_hw(struct net_device *dev)
+{
+	dev->wanted_features &= ~NETIF_F_GRO_HW;
+	netdev_update_features(dev);
+
+	if (unlikely(dev->features & NETIF_F_GRO_HW))
+		netdev_WARN(dev, "failed to disable GRO_HW!\n");
+}
+
 static int call_netdevice_notifier(struct notifier_block *nb, unsigned long val,
 				   struct net_device *dev)
 {

@@ -4564,6 +4581,7 @@ static int generic_xdp_install(struct net_device *dev, struct netdev_bpf *xdp)
 		} else if (new && !old) {
 			static_key_slow_inc(&generic_xdp_needed);
 			dev_disable_lro(dev);
+			dev_disable_gro_hw(dev);
 		}
 		break;
 

-- 
1.8.3.1

Advertise NETIF_F_GRO_HW in hw_features if hardware GRO is supported.
In bnxt_fix_features(), disable GRO_HW and LRO if current hardware
configuration does not allow it.  GRO_HW depends on GRO.  GRO_HW is
also mutually exclusive with LRO.  XDP setup will now rely on
bnxt_fix_features() to turn off aggregation.  During chip init, turn on
or off hardware GRO based on NETIF_F_GRO_HW in features flag.

Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 1d865ae..9efbdc6 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c

@@ -2755,7 +2755,7 @@ void bnxt_set_tpa_flags(struct bnxt *bp)
 		return;
 	if (bp->dev->features & NETIF_F_LRO)
 		bp->flags |= BNXT_FLAG_LRO;
-	if (bp->dev->features & NETIF_F_GRO)
+	else if (bp->dev->features & NETIF_F_GRO_HW)
 		bp->flags |= BNXT_FLAG_GRO;
 }
 

@@ -2843,10 +2843,10 @@ int bnxt_set_rx_skb_mode(struct bnxt *bp, bool page_mode)
 			min_t(u16, bp->max_mtu, BNXT_MAX_PAGE_MODE_MTU);
 		bp->flags &= ~BNXT_FLAG_AGG_RINGS;
 		bp->flags |= BNXT_FLAG_NO_AGG_RINGS | BNXT_FLAG_RX_PAGE_MODE;
-		bp->dev->hw_features &= ~NETIF_F_LRO;
-		bp->dev->features &= ~NETIF_F_LRO;
 		bp->rx_dir = DMA_BIDIRECTIONAL;
 		bp->rx_skb_func = bnxt_rx_page_skb;
+		/* Disable LRO or GRO_HW */
+		netdev_update_features(bp->dev);
 	} else {
 		bp->dev->max_mtu = bp->max_mtu;
 		bp->flags &= ~BNXT_FLAG_RX_PAGE_MODE;

@@ -6788,6 +6788,15 @@ static netdev_features_t bnxt_fix_features(struct net_device *dev,
 	if ((features & NETIF_F_NTUPLE) && !bnxt_rfs_capable(bp))
 		features &= ~NETIF_F_NTUPLE;
 
+	if (bp->flags & BNXT_FLAG_NO_AGG_RINGS)
+		features &= ~(NETIF_F_LRO | NETIF_F_GRO_HW);
+
+	if (!(features & NETIF_F_GRO))
+		features &= ~NETIF_F_GRO_HW;
+
+	if (features & NETIF_F_GRO_HW)
+		features &= ~NETIF_F_LRO;
+
 	/* Both CTAG and STAG VLAN accelaration on the RX side have to be
 	 * turned on or off together.
 	 */

@@ -6821,9 +6830,9 @@ static int bnxt_set_features(struct net_device *dev, netdev_features_t features)
 	bool update_tpa = false;
 
 	flags &= ~BNXT_FLAG_ALL_CONFIG_FEATS;
-	if ((features & NETIF_F_GRO) && !BNXT_CHIP_TYPE_NITRO_A0(bp))
+	if (features & NETIF_F_GRO_HW)
 		flags |= BNXT_FLAG_GRO;
-	if (features & NETIF_F_LRO)
+	else if (features & NETIF_F_LRO)
 		flags |= BNXT_FLAG_LRO;
 
 	if (bp->flags & BNXT_FLAG_NO_AGG_RINGS)

@@ -7924,8 +7933,8 @@ static int bnxt_get_dflt_rings(struct bnxt *bp, int *max_rx, int *max_tx,
 		if (rc)
 			return rc;
 		bp->flags |= BNXT_FLAG_NO_AGG_RINGS;
-		bp->dev->hw_features &= ~NETIF_F_LRO;
-		bp->dev->features &= ~NETIF_F_LRO;
+		bp->dev->hw_features &= ~(NETIF_F_LRO | NETIF_F_GRO_HW);
+		bp->dev->features &= ~(NETIF_F_LRO | NETIF_F_GRO_HW);
 		bnxt_set_ring_params(bp);
 	}
 

@@ -8108,7 +8117,11 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	dev->vlan_features = dev->hw_features | NETIF_F_HIGHDMA;
 	dev->hw_features |= NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_HW_VLAN_CTAG_TX |
 			    NETIF_F_HW_VLAN_STAG_RX | NETIF_F_HW_VLAN_STAG_TX;
+	if (!BNXT_CHIP_TYPE_NITRO_A0(bp))
+		dev->hw_features |= NETIF_F_GRO_HW;
 	dev->features |= dev->hw_features | NETIF_F_HIGHDMA;
+	if (dev->features & NETIF_F_GRO_HW)
+		dev->features &= ~NETIF_F_LRO;
 	dev->priv_flags |= IFF_UNICAST_FLT;
 
 #ifdef CONFIG_BNXT_SRIOV

-- 
1.8.3.1

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 <redacted>
Cc: everest-linux-l2@cavium.com
Signed-off-by: Michael Chan <michael.chan@broadcom.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

Advertise NETIF_F_GRO_HW and set edev->gro_disable according to the
feature flag.  Add qede_fix_features() to drop NETIF_F_GRO_HW if
XDP is running or MTU does not support GRO_HW or GRO is not set.
qede_change_mtu() also checks and disables GRO_HW if MTU is not
supported.

Cc: Ariel Elior <redacted>
Cc: everest-linux-l2@cavium.com
Acked-by: Manish Chopra <redacted>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/qlogic/qede/qede.h         |  2 ++
 drivers/net/ethernet/qlogic/qede/qede_ethtool.c |  3 +++
 drivers/net/ethernet/qlogic/qede/qede_filter.c  | 21 ++++++++++++++-------
 drivers/net/ethernet/qlogic/qede/qede_main.c    | 17 ++++++-----------
 4 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qede/qede.h b/drivers/net/ethernet/qlogic/qede/qede.h
index a3a70ad..8a33651 100644
--- a/drivers/net/ethernet/qlogic/qede/qede.h
+++ b/drivers/net/ethernet/qlogic/qede/qede.h

@@ -494,6 +494,8 @@ int qede_free_tx_pkt(struct qede_dev *edev,
 void qede_vlan_mark_nonconfigured(struct qede_dev *edev);
 int qede_configure_vlan_filters(struct qede_dev *edev);
 
+netdev_features_t qede_fix_features(struct net_device *dev,
+				    netdev_features_t features);
 int qede_set_features(struct net_device *dev, netdev_features_t features);
 void qede_set_rx_mode(struct net_device *ndev);
 void qede_config_rx_mode(struct net_device *ndev);

diff --git a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
index dae7412..4ca3847 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c

@@ -940,6 +940,9 @@ int qede_change_mtu(struct net_device *ndev, int new_mtu)
 	DP_VERBOSE(edev, (NETIF_MSG_IFUP | NETIF_MSG_IFDOWN),
 		   "Configuring MTU size of %d\n", new_mtu);
 
+	if (new_mtu > PAGE_SIZE)
+		ndev->features &= ~NETIF_F_GRO_HW;
+
 	/* Set the mtu field and re-start the interface if needed */
 	args.u.mtu = new_mtu;
 	args.func = &qede_update_mtu;

diff --git a/drivers/net/ethernet/qlogic/qede/qede_filter.c b/drivers/net/ethernet/qlogic/qede/qede_filter.c
index c1a0708..77aa826 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_filter.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_filter.c

@@ -895,19 +895,26 @@ static void qede_set_features_reload(struct qede_dev *edev,
 	edev->ndev->features = args->u.features;
 }
 
+netdev_features_t qede_fix_features(struct net_device *dev,
+				    netdev_features_t features)
+{
+	struct qede_dev *edev = netdev_priv(dev);
+
+	if (edev->xdp_prog || edev->ndev->mtu > PAGE_SIZE ||
+	    !(features & NETIF_F_GRO))
+		features &= ~NETIF_F_GRO_HW;
+
+	return features;
+}
+
 int qede_set_features(struct net_device *dev, netdev_features_t features)
 {
 	struct qede_dev *edev = netdev_priv(dev);
 	netdev_features_t changes = features ^ dev->features;
 	bool need_reload = false;
 
-	/* No action needed if hardware GRO is disabled during driver load */
-	if (changes & NETIF_F_GRO) {
-		if (dev->features & NETIF_F_GRO)
-			need_reload = !edev->gro_disable;
-		else
-			need_reload = edev->gro_disable;
-	}
+	if (changes & NETIF_F_GRO_HW)
+		need_reload = true;
 
 	if (need_reload) {
 		struct qede_reload_args args;

diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index 57332b3..90d79ae 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c

@@ -545,6 +545,7 @@ static int qede_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 #endif
 	.ndo_vlan_rx_add_vid = qede_vlan_rx_add_vid,
 	.ndo_vlan_rx_kill_vid = qede_vlan_rx_kill_vid,
+	.ndo_fix_features = qede_fix_features,
 	.ndo_set_features = qede_set_features,
 	.ndo_get_stats64 = qede_get_stats64,
 #ifdef CONFIG_QED_SRIOV

@@ -572,6 +573,7 @@ static int qede_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 	.ndo_change_mtu = qede_change_mtu,
 	.ndo_vlan_rx_add_vid = qede_vlan_rx_add_vid,
 	.ndo_vlan_rx_kill_vid = qede_vlan_rx_kill_vid,
+	.ndo_fix_features = qede_fix_features,
 	.ndo_set_features = qede_set_features,
 	.ndo_get_stats64 = qede_get_stats64,
 	.ndo_udp_tunnel_add = qede_udp_tunnel_add,

@@ -589,6 +591,7 @@ static int qede_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 	.ndo_change_mtu = qede_change_mtu,
 	.ndo_vlan_rx_add_vid = qede_vlan_rx_add_vid,
 	.ndo_vlan_rx_kill_vid = qede_vlan_rx_kill_vid,
+	.ndo_fix_features = qede_fix_features,
 	.ndo_set_features = qede_set_features,
 	.ndo_get_stats64 = qede_get_stats64,
 	.ndo_udp_tunnel_add = qede_udp_tunnel_add,

@@ -676,7 +679,7 @@ static void qede_init_ndev(struct qede_dev *edev)
 	ndev->priv_flags |= IFF_UNICAST_FLT;
 
 	/* user-changeble features */
-	hw_features = NETIF_F_GRO | NETIF_F_SG |
+	hw_features = NETIF_F_GRO | NETIF_F_GRO_HW | NETIF_F_SG |
 		      NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
 		      NETIF_F_TSO | NETIF_F_TSO6;
 

@@ -1228,18 +1231,9 @@ static int qede_alloc_sge_mem(struct qede_dev *edev, struct qede_rx_queue *rxq)
 	dma_addr_t mapping;
 	int i;
 
-	/* Don't perform FW aggregations in case of XDP */
-	if (edev->xdp_prog)
-		edev->gro_disable = 1;
-
 	if (edev->gro_disable)
 		return 0;
 
-	if (edev->ndev->mtu > PAGE_SIZE) {
-		edev->gro_disable = 1;
-		return 0;
-	}
-
 	for (i = 0; i < ETH_TPA_MAX_AGGS_NUM; i++) {
 		struct qede_agg_info *tpa_info = &rxq->tpa_info[i];
 		struct sw_rx_data *replace_buf = &tpa_info->buffer;

@@ -1269,6 +1263,7 @@ static int qede_alloc_sge_mem(struct qede_dev *edev, struct qede_rx_queue *rxq)
 err:
 	qede_free_sge_mem(edev, rxq);
 	edev->gro_disable = 1;
+	edev->ndev->features &= ~NETIF_F_GRO_HW;
 	return -ENOMEM;
 }
 

@@ -1511,7 +1506,7 @@ static void qede_init_fp(struct qede_dev *edev)
 			 edev->ndev->name, queue_id);
 	}
 
-	edev->gro_disable = !(edev->ndev->features & NETIF_F_GRO);
+	edev->gro_disable = !(edev->ndev->features & NETIF_F_GRO_HW);
 }
 
 static int qede_set_real_num_queues(struct qede_dev *edev)

-- 
1.8.3.1

On Mon, Dec 11, 2017 at 3:41 AM, Michael Chan [off-list ref] wrote:

This last line should probably be updated since we made the change
that dropped the dependency on software GRO.

It might be in our interest here to explain why. Specifically that
"The Rx checksum can no longer be verified once a packet is modified
as a result of GRO_HW, therefore we cannot allow GRO_HW to be enabled
when RXCSUM is disabled." or something to that effect. Otherwise there
isn't a good explanation as to why GRO_HW implies this.

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
 

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

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.

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

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

On Wed, Dec 13, 2017 at 11:46 PM, Chopra, Manish
[off-list ref] wrote:

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 ?

OK.  I will remove GRO.

GRO_HW will be turned off by netdev_fix_features() when RXCSUM is not on.

On Mon, Dec 11, 2017 at 1:41 PM, Michael Chan [off-list ref] wrote:

Introduce NETIF_F_GRO_HW feature flag and convert drivers that support
hardware GRO to use the new flag.

Hi Michael,

Could you add more performance motivations/results? looking on your
netconf slides [1]
I see (much) better CPU utilization (slide 5) -- do you have more
numbers to share?

Or.

[1] http://vger.kernel.org/netconf2017_files/hardware_gro.pdf

On Thu, Dec 14, 2017 at 8:34 AM, Or Gerlitz [off-list ref] wrote:

On Mon, Dec 11, 2017 at 1:41 PM, Michael Chan [off-list ref] wrote:

quoted

Introduce NETIF_F_GRO_HW feature flag and convert drivers that support
hardware GRO to use the new flag.

Hi Michael,

Could you add more performance motivations/results? looking on your
netconf slides [1]
I see (much) better CPU utilization (slide 5) -- do you have more
numbers to share?

The motivations are the same as LRO:  to achieve higher RX throughput
and lower CPU utilization.

You can think of it as an improved version of LRO that is stricter and
provides more information to construct the GRO frame so that it is
reversible.

Or.

[1] http://vger.kernel.org/netconf2017_files/hardware_gro.pdf

On Thu, Dec 14, 2017 at 8:36 PM, Michael Chan [off-list ref] wrote:

On Thu, Dec 14, 2017 at 8:34 AM, Or Gerlitz [off-list ref] wrote:

quoted

On Mon, Dec 11, 2017 at 1:41 PM, Michael Chan [off-list ref] wrote:

quoted

Introduce NETIF_F_GRO_HW feature flag and convert drivers that support
hardware GRO to use the new flag.

Hi Michael,

Could you add more performance motivations/results? looking on your
netconf slides [1]
I see (much) better CPU utilization (slide 5) -- do you have more
numbers to share?

The motivations are the same as LRO:  to achieve higher RX throughput
and lower CPU utilization.

makes sense, what is the +/- %% improvement in RX throughput you see?

You can think of it as an improved version of LRO that is stricter and
provides more information to construct the GRO frame so that it is
reversible.

sounds good to me

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

Michael, I checked it on again, I tried to set LRO in dev->features and dev->hw_features.
Somehow, it gets disabled after register_netdevice().  Any idea why ? Although, I am not running any Bridge/bonding devices.
Looks like, without this series also devices seems to have LRO disabled by default.
Not sure why register_netdevice() disables LRO even driver populates this feature prior to register_netdevice().

On Thu, Dec 14, 2017 at 11:07 PM, Chopra, Manish
[off-list ref] wrote:

Michael, I checked it on again, I tried to set LRO in dev->features and dev->hw_features.
Somehow, it gets disabled after register_netdevice().  Any idea why ? Although, I am not running any Bridge/bonding devices.
Looks like, without this series also devices seems to have LRO disabled by default.
Not sure why register_netdevice() disables LRO even driver populates this feature prior to register_netdevice().

May be you have ip forwarding enabled.