Message-ID: <VI1PR04MB51359EB3614797A89185C9D292B00@VI1PR04MB5135.eurprd04.prod.outlook.com>
Date:   Sat, 29 Dec 2018 01:59:58 +0000
From:   PO LIU <po.liu@....com>
To:     Vinicius Costa Gomes <vinicius.gomes@...el.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "haustad@...co.com" <haustad@...co.com>,
        "nicolas.ferre@...rochip.com" <nicolas.ferre@...rochip.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        Mingkai Hu <mingkai.hu@....com>, Roy Zang <roy.zang@....com>
Subject: RE: [PATCH] net: tsn: add an netlink interface between kernel and
 application layer

Hi Vinicius,

Thank you very much for your feedback.

I know the CBS is used to be most important part of AVB. And qdiscs is good tool to configure qos. 

But as you know, the TSN family is a cluster of protocols and much extending the AVB. The protocols have different  functionalities and they may have more than hundred  parameters. For example NXP ls1028a support Qbv/Qci/Qbu/Qav and also the 8021CB (not included in this patch yet).

Some protocols target to configure the traffic class(like Qav CBS). Some to config the port(like Qbv). But some for the whole ethernet controller(like Qci, the control entries for the whole controller, which input ports and which output ports).

So I do think all the TSN configuration should not mix in the ethernet driver itself. I mean the driver should separate a xxx_tsn.c(for I210, may igb_tsn.c) to maintain the tsn operations. 

As far as using qdiscs or the interface of generic netlink. I think both could configuring the TSN protocols interface layer. Just what I provided the patch net/tsn/genl_tsn.c. But I do believe it is better using a standalone TSN middle layer to maintain the TSN capability ports. Because the TSN ports include not only the end station and also the switch. LS1028 is such a kind of device.

And your advises are precious for us. Let's make out an easy and flexible interface for TSN.

Br,
Po Liu

> -----Original Message-----
> From: Vinicius Costa Gomes [mailto:vinicius.gomes@...el.com]
> Sent: 2018年12月29日 3:30
> To: PO LIU <po.liu@....com>; netdev@...r.kernel.org; linux-
> kernel@...r.kernel.org
> Cc: davem@...emloft.net; haustad@...co.com; nicolas.ferre@...rochip.com;
> gregkh@...uxfoundation.org; Mingkai Hu <mingkai.hu@....com>; Roy Zang
> <roy.zang@....com>; PO LIU <po.liu@....com>; PO LIU <po.liu@....com>
> Subject: Re: [PATCH] net: tsn: add an netlink interface between kernel and
> application layer
> 
> Hi,
> 
> PO LIU <po.liu@....com> writes:
> 
> > This patch provids netlink method to configure the TSN protocols hardwares.
> > TSN guaranteed packet transport with bounded low latency, low packet
> > delay variation, and low packet loss by hardware and software methods.
> 
> I don't think having another way to configure TSN features is a good idea. We
> already have the CBS/ETF/taprio family of qdiscs, that provide (or will in the
> near future, more on this later) a way to configure the hardware.
> 
> A little background on the choice of qdiscs as an interface (and why we came to
> believe they are a good abstraction), they already provide a way to map
> packets into traffic classes (it isn't clear in our proposal how you do that, but I
> think you are using something like mqprio), they provide a neat way to
> "compose" (by installing one under another), they already have a user facing
> API with various counters, and very importantly for TSN they have mecanisms
> to offload some of their work to the hardware.
> 
> I suggest is for you to take a look at how CBS offloading was implemented for
> the Intel i210:
> 
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
> work.ozlabs.org%2Fcover%2F824626%2F&amp;data=02%7C01%7Cpo.liu%40n
> xp.com%7C107e693326214900cf0708d66cfadace%7C686ea1d3bc2b4c6fa92c
> d99c5c301635%7C0%7C0%7C636816222009310445&amp;sdata=6O1s%2Bd%
> 2BbNek1Ko7a5ev0txiw8S6TiVeUrUWZlOXDnAw%3D&amp;reserved=0
> 
> Patches 4 and 5 should be the interesting ones. I think you can use them as
> inspiration for enabling CBS offload in your driver.
> 
> If you did take a look at those patches (and the current work that has been
> upstreamed), my question then becomes, what are the reasons that it might
> not work for your use cases?
> 
> >
> > The three basic components of TSN are:
> >
> > 1. Time synchronization: This was implement by 8021AS which base on the
> >    IEEE1588 precision Time Protocol. This is configured by the other way
> >    in kernel.
> >    8021AS not included in this patch.
> >
> > 2. Scheduling and traffic shaping and per-stream filter policing:
> >    This patch support Qbv/Qci.
> 
> I am working on a proposal for the API for Qbv (and Qbu) offloading using
> taprio. I should send it soon-ish. Your feedback would be very welcome.
> 
> Also, how to expose in the qdiscs the per-stream filtering and policing parts (Qci)
> is something that I don't know how to do right now, any suggestions would be
> nice.
> 
> In short, take a look at what's there and see what's missing for the stuff that
> you care about, then we can work on that.
> 
> >
> > 3. Selection of communication paths:
> >    This patch not support the pure software only TSN protocols(like Qcc)
> >    but hardware related configuration.
> >
> > TSN Protocols supports by this patch: Qbv/Qci/Qbu/Credit-base Shaper(Qav).
> > This patch verified on NXP ls1028ardb board.
> >
> > Will add more protocols in the future.
> >
> > Signed-off-by: Po Liu <Po.Liu@....com>
> 
> 
> Cheers,
> --
> Vinicius

Powered by blists - more mailing lists