Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 30 Mar 2020 15:18:37 -0700
From:   Jakub Kicinski <kuba@kernel.org>
To:     Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
Cc:     Jose.Abreu@synopsys.com, davem@davemloft.net,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        olteanv@gmail.com, ivan.khoronzhuk@linaro.org,
        alexandre.belloni@bootlin.com, andrew@lunn.ch,
        vivien.didelot@gmail.com, po.liu@nxp.com, leoyang.li@nxp.com
Subject: Re: [net-next,v1] net: stmmac: support for tc mqprio offload
Message-ID: <20200330151837.0b5cf804@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
In-Reply-To: <20200328011414.42401-1-xiaoliang.yang_1@nxp.com>
References: <20200328011414.42401-1-xiaoliang.yang_1@nxp.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Sat, 28 Mar 2020 09:14:14 +0800 Xiaoliang Yang wrote:
> This patch add support for tc mqprio offload to configure multiple
> prioritized TX traffic classes with mqprio.
> 
> Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>

> +static int stmmac_tc_setup_mqprio(struct net_device *ndev, void *type_data)
> +{
> +	struct tc_mqprio_qopt *mqprio = type_data;
> +	u8 num_tc;
> +	int i;
> +
> +	num_tc = mqprio->num_tc;

num_tc will be set to 0 when root qdisc is change from mqprio to
something else, I don't see you handling this.

> +	netif_set_real_num_tx_queues(ndev, num_tc);
> +	netdev_set_num_tc(ndev, num_tc);
> +	for (i = 0; i < num_tc; i++)
> +		netdev_set_tc_queue(ndev, i, 1, i);

So you never configure any hardware?

> +	return 0;
> +}
> +
>  static int stmmac_setup_tc(struct net_device *ndev, enum tc_setup_type type,
>  			   void *type_data)
>  {
> @@ -4229,6 +4244,8 @@ static int stmmac_setup_tc(struct net_device *ndev, enum tc_setup_type type,
>  		return stmmac_tc_setup_cbs(priv, priv, type_data);
>  	case TC_SETUP_QDISC_TAPRIO:
>  		return stmmac_tc_setup_taprio(priv, priv, type_data);
> +	case TC_SETUP_QDISC_MQPRIO:
> +		return stmmac_tc_setup_mqprio(ndev, type_data);
>  	case TC_SETUP_QDISC_ETF:
>  		return stmmac_tc_setup_etf(priv, priv, type_data);
>  	default: