RE: [PATCH net-next v2 5/5] qede: Use NETIF_F_GRO_HW.
flat view
From: Chopra, Manish <hidden>
Date: 2017-12-08 17:02:11
quoted hunk ↗ jump to hunk
-----Original Message----- From: Michael Chan [mailto:michael.chan@broadcom.com] Sent: Thursday, December 07, 2017 1:34 PM To: davem@davemloft.net Cc: netdev@vger.kernel.org; andrew.gospodarek@broadcom.com; Elior, Ariel [off-list ref]; Dept-Eng Everest Linux L2 <Dept- EngEverestLinuxL2@cavium.com> Subject: [PATCH net-next v2 5/5] qede: Use NETIF_F_GRO_HW. 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. qede_change_mtu() also checks and disables GRO_HW if MTU is not supported. Cc: Ariel Elior <redacted> Cc: everest-linux-l2@cavium.com 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 | 20 +++++++++++++------- drivers/net/ethernet/qlogic/qede/qede_main.c | 17 ++++++----------- 4 files changed, 24 insertions(+), 18 deletions(-)diff --git a/drivers/net/ethernet/qlogic/qede/qede.hb/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, voidqede_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, intnew_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.cb/drivers/net/ethernet/qlogic/qede/qede_filter.c index c1a0708..2de947e 100644--- a/drivers/net/ethernet/qlogic/qede/qede_filter.c +++ b/drivers/net/ethernet/qlogic/qede/qede_filter.c@@ -895,19 +895,25 @@ static void qede_set_features_reload(structqede_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_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.cb/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
Hi Michael, we have currently tested changes for qede driver and that overall looks good. One small observation though in following steps - 1) HW GRO on initially as per "ethtool -k". 2) change MTU to 9000 which will not be supported with HW gro [HW gro gets off as per "ethtool -k"]. 3) Change MTU back to 1500 which is supported mtu for HW gro but HW gro remains off as per "ethtool -k". I don't have any strong recommendation that in step 3 if user fallback to supported mtu for HW gro then HW gro should be again turned on automatically. I will leave up to user to control it back through ethtool. Please note that we are yet to test bnx2x but I do see you are going to send V3. Hopefully, we will be able to provide feedback on bnx2x by that time. Thanks for making this change. Acked-by: Manish Chopra <redacted>