Messages in this thread | | | From | Wei Fang <> | Subject | RE: [PATCH net-next] net: fec: add CBS offload support | Date | Fri, 10 Feb 2023 08:04:18 +0000 |
| |
> -----Original Message----- > From: Andrew Lunn <andrew@lunn.ch> > Sent: 2023年2月10日 4:38 > To: Wei Fang <wei.fang@nxp.com> > Cc: Shenwei Wang <shenwei.wang@nxp.com>; Clark Wang > <xiaoning.wang@nxp.com>; davem@davemloft.net; edumazet@google.com; > kuba@kernel.org; pabeni@redhat.com; netdev@vger.kernel.org; dl-linux-imx > <linux-imx@nxp.com>; linux-kernel@vger.kernel.org > Subject: Re: [PATCH net-next] net: fec: add CBS offload support > > > + /* cbs->idleslope is in kilobits per second. speed is the port rate > > + * in megabits per second. So bandwidth ratio bw = (idleslope / > > + * (speed * 1000)) * 100, the unit is percentage. > > + */ > > + bw = cbs->idleslope / (speed * 10UL); > > This appears to be a / 0 when the link is not up yet? Also, if the link goes > does, fep->speed keeps the old value, so if it comes up again at a different > speed, your calculations are all wrong. So i think you need > fec_enet_adjust_link() involved in this. > Yes, speed = 0 is indeed a problem, we should check the value first. For speed change, I'll think about how to handle this situation.
> > > + /* bw% can not >= 100% */ > > + if (bw >= 100) > > + return -EINVAL; > > Well > 100% could happen when the link goes from 1G to 10Half, or even > 100Full. You should probably document the policy of what you do then. Do > you dedicate all the available bandwidth to the high priority queue, or do you > go back to best effort? Actually, the FEC has always used the credit-based shaper by default. So I think it's better to fall back the default setting and return error if the bw > 100%.
>Is it possible to indicate in the tc show command that > the configuration is no longer possible? > Sorry, I have no knowledge about the tc show command.
> Presumably other drivers have already addressed all these issues, so you just > need to find out what they do. > > Andrew
| |