Message ID | 1493261053-68197-4-git-send-email-yankejian@huawei.com |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
On 04/26/2017 07:44 PM, Yankejian wrote: > struct hns_nic_priv *priv = netdev_priv(ndev); > struct hnae_ring *ring = ring_data->ring; > @@ -361,6 +361,10 @@ int hns_nic_net_xmit_hw(struct net_device *ndev, > dev_queue = netdev_get_tx_queue(ndev, skb->queue_mapping); > netdev_tx_sent_queue(dev_queue, skb->len); > > + netif_trans_update(ndev); > + ndev->stats.tx_bytes += skb->len; > + ndev->stats.tx_packets++; This is still wrong though, you should not update your TX statistics until you get a TX completion interrupt that confirms these packets were actually transmitted. This has the advantage of not causing use after free in your ndo_start_xmit() function (current bug), and also allows feeding information into BQL where it is appropriate, and in a central location: the TX completion handler.
On 2017/4/28 1:38, Florian Fainelli wrote: > On 04/26/2017 07:44 PM, Yankejian wrote: >> struct hns_nic_priv *priv = netdev_priv(ndev); >> struct hnae_ring *ring = ring_data->ring; >> @@ -361,6 +361,10 @@ int hns_nic_net_xmit_hw(struct net_device *ndev, >> dev_queue = netdev_get_tx_queue(ndev, skb->queue_mapping); >> netdev_tx_sent_queue(dev_queue, skb->len); >> >> + netif_trans_update(ndev); >> + ndev->stats.tx_bytes += skb->len; >> + ndev->stats.tx_packets++; > This is still wrong though, you should not update your TX statistics > until you get a TX completion interrupt that confirms these packets were > actually transmitted. This has the advantage of not causing use after > free in your ndo_start_xmit() function (current bug), and also allows > feeding information into BQL where it is appropriate, and in a central > location: the TX completion handler. Agree, I will fix this, and will delete this patch from patchset and refloat it later.
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c index fca37e2..1e04df6 100644 --- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c +++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c @@ -300,9 +300,9 @@ static void fill_tso_desc(struct hnae_ring *ring, void *priv, mtu); } -int hns_nic_net_xmit_hw(struct net_device *ndev, - struct sk_buff *skb, - struct hns_nic_ring_data *ring_data) +netdev_tx_t hns_nic_net_xmit_hw(struct net_device *ndev, + struct sk_buff *skb, + struct hns_nic_ring_data *ring_data) { struct hns_nic_priv *priv = netdev_priv(ndev); struct hnae_ring *ring = ring_data->ring; @@ -361,6 +361,10 @@ int hns_nic_net_xmit_hw(struct net_device *ndev, dev_queue = netdev_get_tx_queue(ndev, skb->queue_mapping); netdev_tx_sent_queue(dev_queue, skb->len); + netif_trans_update(ndev); + ndev->stats.tx_bytes += skb->len; + ndev->stats.tx_packets++; + wmb(); /* commit all data before submit */ assert(skb->queue_mapping < priv->ae_handle->q_num); hnae_queue_xmit(priv->ae_handle->qs[skb->queue_mapping], buf_num); @@ -1474,17 +1478,11 @@ static netdev_tx_t hns_nic_net_xmit(struct sk_buff *skb, struct net_device *ndev) { struct hns_nic_priv *priv = netdev_priv(ndev); - int ret; assert(skb->queue_mapping < ndev->ae_handle->q_num); - ret = hns_nic_net_xmit_hw(ndev, skb, - &tx_ring_data(priv, skb->queue_mapping)); - if (ret == NETDEV_TX_OK) { - netif_trans_update(ndev); - ndev->stats.tx_bytes += skb->len; - ndev->stats.tx_packets++; - } - return (netdev_tx_t)ret; + + return hns_nic_net_xmit_hw(ndev, skb, + &tx_ring_data(priv, skb->queue_mapping)); } static int hns_nic_change_mtu(struct net_device *ndev, int new_mtu) diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.h b/drivers/net/ethernet/hisilicon/hns/hns_enet.h index 5b412de..7bc6a6e 100644 --- a/drivers/net/ethernet/hisilicon/hns/hns_enet.h +++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.h @@ -91,8 +91,8 @@ struct hns_nic_priv { void hns_nic_net_reset(struct net_device *ndev); void hns_nic_net_reinit(struct net_device *netdev); int hns_nic_init_phy(struct net_device *ndev, struct hnae_handle *h); -int hns_nic_net_xmit_hw(struct net_device *ndev, - struct sk_buff *skb, - struct hns_nic_ring_data *ring_data); +netdev_tx_t hns_nic_net_xmit_hw(struct net_device *ndev, + struct sk_buff *skb, + struct hns_nic_ring_data *ring_data); #endif /**__HNS_ENET_H */