> > > +static void hns3_nic_net_down(struct net_device *ndev) { > > > + struct hns3_nic_priv *priv = netdev_priv(ndev); > > > + struct hnae3_ae_ops *ops; > > > + int i; > > > + > > > + netif_tx_stop_all_queues(ndev); > > > + netif_carrier_off(ndev); > > > + netif_tx_disable(ndev); > > > + > > > + ops = priv->ae_handle->ae_algo->ops; > > > + > > > + if (ops->stop) > > > + ops->stop(priv->ae_handle); > > > + > > > + netif_tx_stop_all_queues(ndev); > > > > Looks a bit excessive. Why do you need all these > > netif_tx_stop_all_queues()? > If we are disabling the netdev. We need to stop scheduling the queues > associated with that netdev for TX, so we need this code. Why do you think > it is excessive?
Why do you need both netif_tx_disable() and netif_tx_stop_all_queues()? And why would you need to re-do netif_tx_stop_all_queues() after calling ops->stop()? > > Isn't mqprio going to override your priority2tc mapping with the one > > provided by user? > I guess you are referring to below code in the mqprio_init() - right? > > static int mqprio_init(struct Qdisc *sch, struct nlattr *opt) > { > [...] > /* Always use supplied priority mappings */ > for (i = 0; i < TC_BITMASK + 1; i++) > netdev_set_prio_tc_map(dev, i, qopt->prio_tc_map[i]); > [...] > } > > In this case yes, you are right below code seems to be redundant: > > + /* Assign UP2TC map for the VSI */ > + for (i = 0; i < HNAE3_MAX_TC; i++) { > + netdev_set_prio_tc_map(ndev, > + kinfo->tc_info[i].up, > + kinfo->tc_info[i].tc); > > Hope I am not missing anything here? You're not; That's what I meant.