[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20210329103925.04db7c9b@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date: Mon, 29 Mar 2021 10:39:25 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Edward Cree <ecree.xilinx@...il.com>
Cc: davem@...emloft.net, netdev@...r.kernel.org,
michael.chan@...adcom.com, damian.dybek@...el.com,
paul.greenwalt@...el.com, rajur@...lsio.com,
jaroslawx.gawin@...el.com, vkochan@...vell.com, alobakin@...me,
snelson@...sando.io, shayagr@...zon.com, ayal@...dia.com,
shenjian15@...wei.com, saeedm@...dia.com, mkubecek@...e.cz,
andrew@...n.ch, roopa@...dia.com
Subject: Re: [PATCH net-next 6/6] ethtool: clarify the ethtool FEC interface
On Mon, 29 Mar 2021 12:56:30 +0100 Edward Cree wrote:
> On 25/03/2021 01:12, Jakub Kicinski wrote:
> > Drivers should reject mixing %ETHTOOL_FEC_AUTO_BIT with other
> > + * FEC modes, because it's unclear whether in this case other modes constrain
> > + * AUTO or are independent choices.
>
> Does this mean you want me to spin a patch to sfc to reject this?
> Currently for us e.g. AUTO|RS means use RS if the cable and link partner
> both support it, otherwise let firmware choose (presumably between BASER
> and OFF) based on cable/module & link partner caps and/or parallel detect.
> We took this approach because our requirements writers believed that
> customers would have a need for this setting; they called it "prefer FEC",
> and I think the idea was to use FEC if possible (even on cables where the
> IEEE-recommended default is no FEC, such as CA-25G-N 3m DAC) but allow
> fallback to no FEC if e.g. link partner doesn't advertise FEC in AN.
> Similarly, AUTO|BASER ("prefer BASE-R FEC") might be desired by a user who
> wants to use BASE-R if possible to minimise latency, but fall back to RS
> FEC if the cable or link partner insists on it (eg CA-25G-L 5m DAC).
> Whether we were right and all this is actually useful, I couldn't say.
Interesting combo. Up to you, the API is quite unclear, I think users
shouldn't expect anything beyond single bit set to work across
implementations. IMHO supporting anything beyond that is just code
complexity for little to no gain. But then again, as long as you don't
confuse AUTO with autoneg there's no burning need to change :)
Powered by blists - more mailing lists