From 2a597eff2437d21841a1e87ffa536ab69dbffdcf Mon Sep 17 00:00:00 2001 From: Yunsheng Lin Date: Thu, 5 Dec 2019 10:12:27 +0800 Subject: [PATCH 1/3] net: hns3: fix for TX queue not restarted problem There is timing window between ring_space checking and netif_stop_subqueue when transmiting a SKB, and the TX BD cleaning may be executed during the time window, which may caused TX queue not restarted problem. This patch fixes it by rechecking the ring_space after netif_stop_subqueue to make sure TX queue is restarted. Also, the ring->next_to_clean is updated even when pkts is zero, because all the TX BD cleaned may be non-SKB, so it needs to check if TX queue need to be restarted. Fixes: 76ad4f0ee747 ("net: hns3: Add support of HNS3 Ethernet Driver for hip08 SoC") Signed-off-by: Yunsheng Lin Signed-off-by: Huazhong Tan Signed-off-by: David S. Miller --- .../net/ethernet/hisilicon/hns3/hns3_enet.c | 33 ++++++++++++------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c index ba0536802b13..e2730319bd43 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c @@ -1287,8 +1287,10 @@ static bool hns3_skb_need_linearized(struct sk_buff *skb, unsigned int *bd_size, } static int hns3_nic_maybe_stop_tx(struct hns3_enet_ring *ring, + struct net_device *netdev, struct sk_buff **out_skb) { + struct hns3_nic_priv *priv = netdev_priv(netdev); unsigned int bd_size[HNS3_MAX_TSO_BD_NUM + 1U]; struct sk_buff *skb = *out_skb; unsigned int bd_num; @@ -1320,10 +1322,23 @@ static int hns3_nic_maybe_stop_tx(struct hns3_enet_ring *ring, } out: - if (unlikely(ring_space(ring) < bd_num)) - return -EBUSY; + if (likely(ring_space(ring) >= bd_num)) + return bd_num; - return bd_num; + netif_stop_subqueue(netdev, ring->queue_index); + smp_mb(); /* Memory barrier before checking ring_space */ + + /* Start queue in case hns3_clean_tx_ring has just made room + * available and has not seen the queue stopped state performed + * by netif_stop_subqueue above. + */ + if (ring_space(ring) >= bd_num && netif_carrier_ok(netdev) && + !test_bit(HNS3_NIC_STATE_DOWN, &priv->state)) { + netif_start_subqueue(netdev, ring->queue_index); + return bd_num; + } + + return -EBUSY; } static void hns3_clear_desc(struct hns3_enet_ring *ring, int next_to_use_orig) @@ -1400,13 +1415,13 @@ netdev_tx_t hns3_nic_net_xmit(struct sk_buff *skb, struct net_device *netdev) /* Prefetch the data used later */ prefetch(skb->data); - ret = hns3_nic_maybe_stop_tx(ring, &skb); + ret = hns3_nic_maybe_stop_tx(ring, netdev, &skb); if (unlikely(ret <= 0)) { if (ret == -EBUSY) { u64_stats_update_begin(&ring->syncp); ring->stats.tx_busy++; u64_stats_update_end(&ring->syncp); - goto out_net_tx_busy; + return NETDEV_TX_BUSY; } else if (ret == -ENOMEM) { u64_stats_update_begin(&ring->syncp); ring->stats.sw_err_cnt++; @@ -1457,12 +1472,6 @@ fill_err: out_err_tx_ok: dev_kfree_skb_any(skb); return NETDEV_TX_OK; - -out_net_tx_busy: - netif_stop_subqueue(netdev, ring->queue_index); - smp_mb(); /* Commit all data before submit */ - - return NETDEV_TX_BUSY; } static int hns3_nic_net_set_mac_address(struct net_device *netdev, void *p) @@ -2519,7 +2528,7 @@ void hns3_clean_tx_ring(struct hns3_enet_ring *ring) dev_queue = netdev_get_tx_queue(netdev, ring->tqp->tqp_index); netdev_tx_completed_queue(dev_queue, pkts, bytes); - if (unlikely(pkts && netif_carrier_ok(netdev) && + if (unlikely(netif_carrier_ok(netdev) && ring_space(ring) > HNS3_MAX_TSO_BD_NUM)) { /* Make sure that anybody stopping the queue after this * sees the new next_to_clean. From d1a37dedcfcf2c01daff5281c3c378876a04e2f4 Mon Sep 17 00:00:00 2001 From: Yunsheng Lin Date: Thu, 5 Dec 2019 10:12:28 +0800 Subject: [PATCH 2/3] net: hns3: fix a use after free problem in hns3_nic_maybe_stop_tx() Currently, hns3_nic_maybe_stop_tx() uses skb_copy() to linearize a SKB if the BD num required by the SKB does not meet the hardware limitation, and it linearizes the SKB by allocating a new linearized SKB and freeing the old SKB, if hns3_nic_maybe_stop_tx() returns -EBUSY because there are no enough space in the ring to send the linearized skb to hardware, the sch_direct_xmit() still hold reference to old SKB and try to retransmit the old SKB when dev_hard_start_xmit() return TX_BUSY, which may cause use after freed problem. This patch fixes it by using __skb_linearize() to linearize the SKB in hns3_nic_maybe_stop_tx(). Fixes: 51e8439f3496 ("net: hns3: add 8 BD limit for tx flow") Signed-off-by: Yunsheng Lin Signed-off-by: Huazhong Tan Signed-off-by: David S. Miller --- .../net/ethernet/hisilicon/hns3/hns3_enet.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c index e2730319bd43..69545dd6c938 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c @@ -1288,31 +1288,24 @@ static bool hns3_skb_need_linearized(struct sk_buff *skb, unsigned int *bd_size, static int hns3_nic_maybe_stop_tx(struct hns3_enet_ring *ring, struct net_device *netdev, - struct sk_buff **out_skb) + struct sk_buff *skb) { struct hns3_nic_priv *priv = netdev_priv(netdev); unsigned int bd_size[HNS3_MAX_TSO_BD_NUM + 1U]; - struct sk_buff *skb = *out_skb; unsigned int bd_num; bd_num = hns3_tx_bd_num(skb, bd_size); if (unlikely(bd_num > HNS3_MAX_NON_TSO_BD_NUM)) { - struct sk_buff *new_skb; - if (bd_num <= HNS3_MAX_TSO_BD_NUM && skb_is_gso(skb) && !hns3_skb_need_linearized(skb, bd_size, bd_num)) goto out; - /* manual split the send packet */ - new_skb = skb_copy(skb, GFP_ATOMIC); - if (!new_skb) + if (__skb_linearize(skb)) return -ENOMEM; - dev_kfree_skb_any(skb); - *out_skb = new_skb; - bd_num = hns3_tx_bd_count(new_skb->len); - if ((skb_is_gso(new_skb) && bd_num > HNS3_MAX_TSO_BD_NUM) || - (!skb_is_gso(new_skb) && + bd_num = hns3_tx_bd_count(skb->len); + if ((skb_is_gso(skb) && bd_num > HNS3_MAX_TSO_BD_NUM) || + (!skb_is_gso(skb) && bd_num > HNS3_MAX_NON_TSO_BD_NUM)) return -ENOMEM; @@ -1415,7 +1408,7 @@ netdev_tx_t hns3_nic_net_xmit(struct sk_buff *skb, struct net_device *netdev) /* Prefetch the data used later */ prefetch(skb->data); - ret = hns3_nic_maybe_stop_tx(ring, netdev, &skb); + ret = hns3_nic_maybe_stop_tx(ring, netdev, skb); if (unlikely(ret <= 0)) { if (ret == -EBUSY) { u64_stats_update_begin(&ring->syncp); From 1c9855085ebaaa2efcf368bee038e4811015abbd Mon Sep 17 00:00:00 2001 From: Jian Shen Date: Thu, 5 Dec 2019 10:12:29 +0800 Subject: [PATCH 3/3] net: hns3: fix VF ID issue for setting VF VLAN Previously, when set VF VLAN with command "ip link set vf vlan ", the VF ID 0 is handled as PF incorrectly, which should be the first VF. This patch fixes it. Fixes: 21e043cd8124 ("net: hns3: fix set port based VLAN for PF") Signed-off-by: Jian Shen Signed-off-by: Huazhong Tan Signed-off-by: David S. Miller --- .../hisilicon/hns3/hns3pf/hclge_main.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c index 7c7038676d6d..d862e9ba27e1 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c @@ -8438,13 +8438,16 @@ static int hclge_set_vf_vlan_filter(struct hnae3_handle *handle, int vfid, if (hdev->pdev->revision == 0x20) return -EOPNOTSUPP; + vport = hclge_get_vf_vport(hdev, vfid); + if (!vport) + return -EINVAL; + /* qos is a 3 bits value, so can not be bigger than 7 */ - if (vfid >= hdev->num_alloc_vfs || vlan > VLAN_N_VID - 1 || qos > 7) + if (vlan > VLAN_N_VID - 1 || qos > 7) return -EINVAL; if (proto != htons(ETH_P_8021Q)) return -EPROTONOSUPPORT; - vport = &hdev->vport[vfid]; state = hclge_get_port_base_vlan_state(vport, vport->port_base_vlan_cfg.state, vlan); @@ -8455,21 +8458,12 @@ static int hclge_set_vf_vlan_filter(struct hnae3_handle *handle, int vfid, vlan_info.qos = qos; vlan_info.vlan_proto = ntohs(proto); - /* update port based VLAN for PF */ - if (!vfid) { - hclge_notify_client(hdev, HNAE3_DOWN_CLIENT); - ret = hclge_update_port_base_vlan_cfg(vport, state, &vlan_info); - hclge_notify_client(hdev, HNAE3_UP_CLIENT); - - return ret; - } - if (!test_bit(HCLGE_VPORT_STATE_ALIVE, &vport->state)) { return hclge_update_port_base_vlan_cfg(vport, state, &vlan_info); } else { ret = hclge_push_vf_port_base_vlan_info(&hdev->vport[0], - (u8)vfid, state, + vport->vport_id, state, vlan, qos, ntohs(proto)); return ret;