* Re: No traffic with Marvell switch and latest linux-next
From: Russell King - ARM Linux admin @ 2019-02-24 19:41 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Heiner Kallweit, Florian Fainelli, netdev@vger.kernel.org
In-Reply-To: <20190224172848.GI26626@lunn.ch>
On Sun, Feb 24, 2019 at 06:28:48PM +0100, Andrew Lunn wrote:
> On Sun, Feb 24, 2019 at 03:31:26PM +0000, Russell King - ARM Linux admin wrote:
> > On Sun, Feb 24, 2019 at 12:42:35AM +0100, Andrew Lunn wrote:
> > > Looking forward, at some point we are going to have to make fixed-link
> > > support higher speeds. That probably means we need a swphy-c45 which
> > > emulates the standard registers for 2.5G, 5G and 10G. At that point
> > > genphy will not work...
> >
> > Do we _need_ to emulate Clause 45 PHYs?
>
> Hi Russell
>
> One use case would be a mv88e6390X port 9 or 10 connected to a SoC
> which can only do 2.5G.
>
> We have defined that DSA drivers should configure CPU and DSA ports to
> their maximum speed. So if port 9 or 10 is used, it should be
> configured to 10G.
>
> We then need some way to reconfigure the MAC to a slower speed.
>
> We need to do this with ZII boards. We do this with:
>
> port@0 {
> reg = <0>;
> label = "cpu";
> ethernet = <&fec1>;
>
> fixed-link {
> speed = <100>;
> full-duplex;
> };
> };
>
> The FEC can only do 100Mbs, but the switch defaults to 1G. So the
> fixed link it used to tell the switch MAC to use 100/Full.
>
> In the example of the 6390X, we would want to set the link speed to
> 2500, which we cannot do at the moment. Either we need fixed-link to
> support higher speeds, or we need a different mechanism.
>
> We also have a similar issue on the SoC side. The FEC has no PHY
> connected to it. It needs to be told what speed to do:
>
> &fec1 {
> phy-mode = "rmii";
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_fec1>;
> status = "okay";
>
> fixed-link {
> speed = <100>;
> full-duplex;
> };
>
> In the case of a SoC with an interface which can do 2.5G, you need to
> tell it to do 2.5G. Ideally we want a mechanism that allows a MAC to
> 'see' a PHY operating at 2.5G using the standard phylib/phylink API.
>
> In the past this has been achieved with an emulated PHY. But so long
> as the phydev/phylink structure has the correct values, it does not
> matter how they get those values.
You'll get a mac_config() with the mode set to MLO_AN_FIXED.
state->speed set to whatever was set in the "speed =" and state->duplex
set according to the duplex property. If pause modes are also defined,
you'll get those through state->pause MLO_PAUSE_SYM and MLO_PAUSE_ASYM,
but there won't be any resolution of those modes since a fixed link
doesn't know the other ends properties, and of course AN is disabled.
Apart from the pause modes, the information passed to the MAC for a
fixed link is exactly the same as if it came from a PHY in phylib,
which basically means the MAC side doesn't actually need to care
whether it has a PHY or fixed-link - but without the complication of
needing phylib to provide an emulated PHY to the network driver.
phylink does provide an emulated PHY for the mii ioctls since that is
an established interface for speeds <= 1G.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* Re: [PATCH ethtool] ethtool: Add support for 200Gbps (50Gbps per lane) link mode
From: Andrew Lunn @ 2019-02-24 19:40 UTC (permalink / raw)
To: Michal Kubecek
Cc: Tariq Toukan, John W. Linville, netdev, Eran Ben Elisha,
Aya Levin
In-Reply-To: <20190224184046.GB1914@unicorn.suse.cz>
> > This is getting less friendly all the time, and it was never very
> > friendly to start with. We have the strings which represent these link
> > modes in the table used for dumping caps. How about allowing the user
> > to list a comma separate list of modes.
> >
> > ethtool -s lan42 advertise 100000baseKR2/Full,100000baseSR2/Full,100000baseCR2/Full
>
> In my preliminary netlink patchset, I'm adding support for e.g.
>
> ethtool -s eth0 advertise 100baseT/Half off 1000baseT/Full on
>
> I'm not sure what would be more useful, if switching individual modes or
> listing the whole set. After all, we can also support both. But I fully
> agree that the numeric bitmaps are already too inconvenient.
Hi Michal
So are you doing a read/modify/write? In that case, off/on makes
sense. For a pure write, i don't see the need for off/on.
I've not had to use this much, so i don't know how it is typically
used. When i have used it, it is because i've got an SFP which can do
1G and 2.5G, but the peer can only do 1G. I've needed to remove the
2.5G in order to get link. So in that case, read/modify/write with an
off would make sense.
Andrew
> And there is another problem:
>
> + else if (speed_wanted == SPEED_20000 &&
> + duplex_wanted == DUPLEX_FULL)
> + adv_bit = ETHTOOL_LINK_MODE_200000baseKR4_Full_BIT &
> + ETHTOOL_LINK_MODE_200000baseSR4_Full_BIT &
> + ETHTOOL_LINK_MODE_200000baseLR4_ER4_FR4_Full_BIT &
> + ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT &
> + ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT;
>
> The test is for SPEED_20000 but then 200G modes are added.
Oh, yes. Easy to miss. Maybe we should consider adding aliases,
#define SPEED_200G SPEED_200000, and
#define ETHTOOL_LINK_MODE_200GbaseCR4_Full_BIT ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT
Andrew
^ permalink raw reply
* Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
From: Andy Lutomirski @ 2019-02-24 19:35 UTC (permalink / raw)
To: Nadav Amit
Cc: Andy Lutomirski, Jann Horn, Alexei Starovoitov, Steven Rostedt,
Linus Torvalds, Masami Hiramatsu, Linux List Kernel Mailing,
Ingo Molnar, Andrew Morton, Changbin Du, Kees Cook,
Daniel Borkmann, Network Development, bpf@vger.kernel.org,
Rick Edgecombe, Dave Hansen, Peter Zijlstra (Intel), Igor Stoppa
In-Reply-To: <7E4E5362-4C13-40FA-B5A8-F88D3E60827F@vmware.com>
On Sat, Feb 23, 2019 at 12:30 AM Nadav Amit <namit@vmware.com> wrote:
>
> > On Feb 22, 2019, at 3:59 PM, Andy Lutomirski <luto@kernel.org> wrote:
> >
> > On Fri, Feb 22, 2019 at 3:02 PM Jann Horn <jannh@google.com> wrote:
> >> On Fri, Feb 22, 2019 at 11:39 PM Nadav Amit <namit@vmware.com> wrote:
> >>>> On Feb 22, 2019, at 2:21 PM, Nadav Amit <namit@vmware.com> wrote:
> >>>>
> >>>>> On Feb 22, 2019, at 2:17 PM, Jann Horn <jannh@google.com> wrote:
> >>>>>
> >>>>> On Fri, Feb 22, 2019 at 11:08 PM Nadav Amit <namit@vmware.com> wrote:
> >>>>>>> On Feb 22, 2019, at 1:43 PM, Jann Horn <jannh@google.com> wrote:
> >>>>>>>
> >>>>>>> (adding some people from the text_poke series to the thread, removing stable@)
> >>>>>>>
> >>>>>>> On Fri, Feb 22, 2019 at 8:55 PM Andy Lutomirski <luto@amacapital.net> wrote:
> >>>>>>>>> On Feb 22, 2019, at 11:34 AM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> >>>>>>>>>> On Fri, Feb 22, 2019 at 02:30:26PM -0500, Steven Rostedt wrote:
> >>>>>>>>>> On Fri, 22 Feb 2019 11:27:05 -0800
> >>>>>>>>>> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> >>>>>>>>>>
> >>>>>>>>>>>> On Fri, Feb 22, 2019 at 09:43:14AM -0800, Linus Torvalds wrote:
> >>>>>>>>>>>>
> >>>>>>>>>>>> Then we should still probably fix up "__probe_kernel_read()" to not
> >>>>>>>>>>>> allow user accesses. The easiest way to do that is actually likely to
> >>>>>>>>>>>> use the "unsafe_get_user()" functions *without* doing a
> >>>>>>>>>>>> uaccess_begin(), which will mean that modern CPU's will simply fault
> >>>>>>>>>>>> on a kernel access to user space.
> >>>>>>>>>>>
> >>>>>>>>>>> On bpf side the bpf_probe_read() helper just calls probe_kernel_read()
> >>>>>>>>>>> and users pass both user and kernel addresses into it and expect
> >>>>>>>>>>> that the helper will actually try to read from that address.
> >>>>>>>>>>>
> >>>>>>>>>>> If __probe_kernel_read will suddenly start failing on all user addresses
> >>>>>>>>>>> it will break the expectations.
> >>>>>>>>>>> How do we solve it in bpf_probe_read?
> >>>>>>>>>>> Call probe_kernel_read and if that fails call unsafe_get_user byte-by-byte
> >>>>>>>>>>> in the loop?
> >>>>>>>>>>> That's doable, but people already complain that bpf_probe_read() is slow
> >>>>>>>>>>> and shows up in their perf report.
> >>>>>>>>>>
> >>>>>>>>>> We're changing kprobes to add a specific flag to say that we want to
> >>>>>>>>>> differentiate between kernel or user reads. Can this be done with
> >>>>>>>>>> bpf_probe_read()? If it's showing up in perf report, I doubt a single
> >>>>>>>>>
> >>>>>>>>> so you're saying you will break existing kprobe scripts?
> >>>>>>>>> I don't think it's a good idea.
> >>>>>>>>> It's not acceptable to break bpf_probe_read uapi.
> >>>>>>>>
> >>>>>>>> If so, the uapi is wrong: a long-sized number does not reliably identify an address if you don’t separately know whether it’s a user or kernel address. s390x and 4G:4G x86_32 are the notable exceptions. I have lobbied for RISC-V and future x86_64 to join the crowd. I don’t know whether I’ll win this fight, but the uapi will probably have to change for at least s390x.
> >>>>>>>>
> >>>>>>>> What to do about existing scripts is a different question.
> >>>>>>>
> >>>>>>> This lack of logical separation between user and kernel addresses
> >>>>>>> might interact interestingly with the text_poke series, specifically
> >>>>>>> "[PATCH v3 05/20] x86/alternative: Initialize temporary mm for
> >>>>>>> patching" (https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20190221234451.17632-6-rick.p.edgecombe%40intel.com%2F&data=02%7C01%7Cnamit%40vmware.com%7Cbab53e52cc5c4ac4419008d69921d1f1%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636864767879731941&sdata=2tqD7udTCfNbcNLcj5SFpZt8WwK5NwtgaWMKm1Ye1EE%3D&reserved=0)
> >>>>>>> and "[PATCH v3 06/20] x86/alternative: Use temporary mm for text
> >>>>>>> poking" (https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20190221234451.17632-7-rick.p.edgecombe%40intel.com%2F&data=02%7C01%7Cnamit%40vmware.com%7Cbab53e52cc5c4ac4419008d69921d1f1%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636864767879731941&sdata=7%2BLShgLKnra6xzSkxdJrCclCacfdE5IdczwScW83nuE%3D&reserved=0),
> >>>>>>> right? If someone manages to get a tracing BPF program to trigger in a
> >>>>>>> task that has switched to the patching mm, could they use
> >>>>>>> bpf_probe_write_user() - which uses probe_kernel_write() after
> >>>>>>> checking that KERNEL_DS isn't active and that access_ok() passes - to
> >>>>>>> overwrite kernel text that is mapped writable in the patching mm?
> >>>>>>
> >>>>>> Yes, this is a good point. I guess text_poke() should be defined with
> >>>>>> “__kprobes” and open-code memcpy.
> >>>>>>
> >>>>>> Does it sound reasonable?
> >>>>>
> >>>>> Doesn't __text_poke() as implemented in the proposed patch use a
> >>>>> couple other kernel functions, too? Like switch_mm_irqs_off() and
> >>>>> pte_clear() (which can be a call into a separate function on paravirt
> >>>>> kernels)?
> >>>>
> >>>> I will move the pte_clear() to be done after the poking mm was unloaded.
> >>>> Give me a few minutes to send a sketch of what I think should be done.
> >>>
> >>> Err.. You are right, I don’t see an easy way of preventing a kprobe from
> >>> being set on switch_mm_irqs_off(), and open-coding this monster is too ugly.
> >>>
> >>> The reasonable solution seems to me as taking all the relevant pieces of
> >>> code (and data) that might be used during text-poking and encapsulating them, so they
> >>> will be set in a memory area which cannot be kprobe'd. This can also be
> >>> useful to write-protect data structures of code that calls text_poke(),
> >>> e.g., static-keys. It can also protect data on that stack that is used
> >>> during text_poke() from being overwritten from another core.
> >>>
> >>> This solution is somewhat similar to Igor Stoppa’s idea of using “enclaves”
> >>> when doing write-rarely operations.
> >>>
> >>> Right now, I think that text_poke() will keep being susceptible to such
> >>> an attack, unless you have a better suggestion.
> >>
> >> A relatively simple approach might be to teach BPF not to run kprobe
> >> programs and such in contexts where current->mm isn't the active mm?
> >> Maybe using nmi_uaccess_okay(), or something like that? It looks like
> >> perf_callchain_user() also already uses that. Except that a lot of
> >> this code is x86-specific...
> >
> > This sounds like exactly the right solution. If you're running from
> > some unknown context (like NMI or tracing), then you should check
> > nmi_uaccess_okay(). I think we should just promote that to be a
> > non-arch-specific function (that returns true by default) and check it
> > the relevant bpf_probe_xyz() functions.
>
> I can do that, but notice that switch_mm_irqs_off() writes to
> cpu_tlbstate.loaded_mm before it actually writes to CR3. So there are still
> a couple of instructions (and the load_new_mm_cr3()) in between that a
> kprobe can be set on, no?
But you can't mark then as no-nmi :) See the comment in
nmi_uaccess_ok() -- the code is intended to work correctly during this
window.
^ permalink raw reply
* Re: [PATCH] Documentation: networking: switchdev: Update port parent ID section
From: David Miller @ 2019-02-24 19:26 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev, andrew, vivien.didelot, idosch, jiri
In-Reply-To: <c796b361-d19a-1b73-ea17-2cbedb10f241@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Sun, 24 Feb 2019 08:46:59 -0800
> Looks like you applied this to the "net" tree, and I missed prefixing
> the patch to indicate this was targeted at the "net-next" tree. Are you
> planning a merge from "net" to "net-next" anytime soon?
Yes, I am working on that right now.
^ permalink raw reply
* Re: [PATCH ethtool] ethtool: Add support for 200Gbps (50Gbps per lane) link mode
From: Michal Kubecek @ 2019-02-24 18:40 UTC (permalink / raw)
To: Andrew Lunn
Cc: Tariq Toukan, John W. Linville, netdev, Eran Ben Elisha,
Aya Levin
In-Reply-To: <20190224164751.GG26626@lunn.ch>
On Sun, Feb 24, 2019 at 05:47:51PM +0100, Andrew Lunn wrote:
> On Sun, Feb 24, 2019 at 05:08:21PM +0200, Tariq Toukan wrote:
> > From: Aya Levin <ayal@mellanox.com>
> > index 5a26cff5fb33..64ce0711ad5f 100644
> > --- a/ethtool.8.in
> > +++ b/ethtool.8.in
> > @@ -650,6 +650,11 @@ lB l lB.
> > 0x400000000 50000baseCR2 Full
> > 0x800000000 50000baseKR2 Full
> > 0x10000000000 50000baseSR2 Full
> > +0x10000000000000 50000baseKR Full
> > +0x20000000000000 50000baseSR Full
> > +0x40000000000000 50000baseCR Full
> > +0x80000000000000 50000baseLR_ER_FR Full
> > +0x100000000000000 50000baseDR Full
> > 0x8000000 56000baseKR4 Full
> > 0x10000000 56000baseCR4 Full
> > 0x20000000 56000baseSR4 Full
> > @@ -658,6 +663,16 @@ lB l lB.
> > 0x2000000000 100000baseSR4 Full
> > 0x4000000000 100000baseCR4 Full
> > 0x8000000000 100000baseLR4_ER4 Full
> > +0x200000000000000 100000baseKR2 Full
> > +0x400000000000000 100000baseSR2 Full
> > +0x800000000000000 100000baseCR2 Full
> > +0x1000000000000000 100000baseLR2_ER2_FR2 Full
> > +0x2000000000000000 100000baseDR2 Full
> > +0x4000000000000000 200000baseKR4 Full
> > +0x8000000000000000 200000baseSR4 Full
> > +0x10000000000000000 200000baseLR4_ER4_FR4 Full
> > +0x20000000000000000 200000baseDR4 Full
> > +0x40000000000000000 200000baseCR4 Full
>
> This is getting less friendly all the time, and it was never very
> friendly to start with. We have the strings which represent these link
> modes in the table used for dumping caps. How about allowing the user
> to list a comma separate list of modes.
>
> ethtool -s lan42 advertise 100000baseKR2/Full,100000baseSR2/Full,100000baseCR2/Full
In my preliminary netlink patchset, I'm adding support for e.g.
ethtool -s eth0 advertise 100baseT/Half off 1000baseT/Full on
I'm not sure what would be more useful, if switching individual modes or
listing the whole set. After all, we can also support both. But I fully
agree that the numeric bitmaps are already too inconvenient.
> > + adv_bit = ETHTOOL_LINK_MODE_200000baseKR4_Full_BIT &
> > + ETHTOOL_LINK_MODE_200000baseSR4_Full_BIT &
> > + ETHTOOL_LINK_MODE_200000baseLR4_ER4_FR4_Full_BIT &
> > + ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT &
> > + ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT;
>
> Maybe i'm wrong, but this looks odd.
>
> enum ethtool_link_mode_bit_indices {
> ETHTOOL_LINK_MODE_10baseT_Half_BIT = 0,
> ETHTOOL_LINK_MODE_10baseT_Full_BIT = 1,
> ETHTOOL_LINK_MODE_100baseT_Half_BIT = 2,
> ETHTOOL_LINK_MODE_100baseT_Full_BIT = 3,
> ETHTOOL_LINK_MODE_1000baseT_Half_BIT = 4,
> ETHTOOL_LINK_MODE_1000baseT_Full_BIT = 5,
>
> These are numbers, not bitmasks, so & them together does not look
> correct.
Yes, this is wrong. Even if bit masks were used, the operator should be
"|" but here adv_bit is interpreted as an index, not mask. It's obvious
this part of the code was designed when speed and duplex identified the
mode uniquely which is no longer the case. (Which is probably also why
there are no branches for speeds over 10G.)
And there is another problem:
+ else if (speed_wanted == SPEED_20000 &&
+ duplex_wanted == DUPLEX_FULL)
+ adv_bit = ETHTOOL_LINK_MODE_200000baseKR4_Full_BIT &
+ ETHTOOL_LINK_MODE_200000baseSR4_Full_BIT &
+ ETHTOOL_LINK_MODE_200000baseLR4_ER4_FR4_Full_BIT &
+ ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT &
+ ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT;
The test is for SPEED_20000 but then 200G modes are added.
Michal
^ permalink raw reply
* Re: [GIT] Networking
From: pr-tracker-bot @ 2019-02-24 18:15 UTC (permalink / raw)
To: David Miller; +Cc: torvalds, akpm, netdev, linux-kernel
In-Reply-To: <20190223.185710.866316846403242352.davem@davemloft.net>
The pull request you sent on Sat, 23 Feb 2019 18:57:10 -0800 (PST):
> git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git refs/heads/master
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/c4eb1e1852df60d61e04a6b580a0490460c9e31b
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* Re: [PATCH ethtool] ethtool: Add support for 200Gbps (50Gbps per lane) link mode
From: Michal Kubecek @ 2019-02-24 18:11 UTC (permalink / raw)
To: Tariq Toukan; +Cc: John W. Linville, netdev, Eran Ben Elisha, Aya Levin
In-Reply-To: <1551020901-20257-1-git-send-email-tariqt@mellanox.com>
On Sun, Feb 24, 2019 at 05:08:21PM +0200, Tariq Toukan wrote:
> From: Aya Levin <ayal@mellanox.com>
>
> Introduce 50Gbps per lane link modes and 200Gbps speed, update print
> functions and initialization functions accordingly.
> In addition, update related man page accordingly.
>
> Signed-off-by: Aya Levin <ayal@mellanox.com>
> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
> ---
> ethtool-copy.h | 19 ++++++++++++++++++-
> ethtool.8.in | 15 +++++++++++++++
> ethtool.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 85 insertions(+), 1 deletion(-)
>
> diff --git a/ethtool-copy.h b/ethtool-copy.h
> index 6bfbb85f9402..de459900b2d3 100644
> --- a/ethtool-copy.h
> +++ b/ethtool-copy.h
> @@ -1455,15 +1455,31 @@ enum ethtool_link_mode_bit_indices {
> ETHTOOL_LINK_MODE_FEC_NONE_BIT = 49,
> ETHTOOL_LINK_MODE_FEC_RS_BIT = 50,
> ETHTOOL_LINK_MODE_FEC_BASER_BIT = 51,
> + ETHTOOL_LINK_MODE_50000baseKR_Full_BIT = 52,
> + ETHTOOL_LINK_MODE_50000baseSR_Full_BIT = 53,
> + ETHTOOL_LINK_MODE_50000baseCR_Full_BIT = 54,
> + ETHTOOL_LINK_MODE_50000baseLR_ER_FR_Full_BIT = 55,
> + ETHTOOL_LINK_MODE_50000baseDR_Full_BIT = 56,
> + ETHTOOL_LINK_MODE_100000baseKR2_Full_BIT = 57,
> + ETHTOOL_LINK_MODE_100000baseSR2_Full_BIT = 58,
> + ETHTOOL_LINK_MODE_100000baseCR2_Full_BIT = 59,
> + ETHTOOL_LINK_MODE_100000baseLR2_ER2_FR2_Full_BIT = 60,
> + ETHTOOL_LINK_MODE_100000baseDR2_Full_BIT = 61,
> + ETHTOOL_LINK_MODE_200000baseKR4_Full_BIT = 62,
> + ETHTOOL_LINK_MODE_200000baseSR4_Full_BIT = 63,
> + ETHTOOL_LINK_MODE_200000baseLR4_ER4_FR4_Full_BIT = 64,
> + ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT = 65,
> + ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT = 66,
>
> /* Last allowed bit for __ETHTOOL_LINK_MODE_LEGACY_MASK is bit
> * 31. Please do NOT define any SUPPORTED_* or ADVERTISED_*
> * macro for bits > 31. The only way to use indices > 31 is to
> * use the new ETHTOOL_GLINKSETTINGS/ETHTOOL_SLINKSETTINGS API.
> */
> + ETHTOOL_LINK_MODE_LAST,
>
> __ETHTOOL_LINK_MODE_LAST
> - = ETHTOOL_LINK_MODE_FEC_BASER_BIT,
> + = ETHTOOL_LINK_MODE_LAST - 1,
> };
The name ETHTOOL_LINK_MODE_LAST is a bit misleading, maybe it should
rather be called ETHTOOL_LINK_MODE_COUNT. Also, there should be
parentheses around the expression to avoid surprises when
__ETHTOOL_LINK_MODE_LAST is used in an expression.
But this change is not in kernel include/uapi/linux/ethtool.h in
mainline, net or net-next. As ethtool-copy.h is supposed to be a copy of
sanitized kernel uapi header, you should make the change there first and
then sync the header to ethtool.
Michal Kubecek
^ permalink raw reply
* Re: [PATCH net] net: phy: fix reading fixed phy status
From: Heiner Kallweit @ 2019-02-24 18:00 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org
In-Reply-To: <20190224175710.GL26626@lunn.ch>
On 24.02.2019 18:57, Andrew Lunn wrote:
> On Sun, Feb 24, 2019 at 06:01:18PM +0100, Heiner Kallweit wrote:
>> With the switch to phy_resolve_aneg_linkmode() we don't read from the
>> chip any longer what is advertised but use phydev->advertising directly.
>> For a fixed phy however this bitmap is empty so far, what results in
>> no common mode being found. This breaks DSA. Fix this by advertising
>> everything that is supported. For a normal phy this done by phy_probe().
>>
>> Fixes: 5502b218e001 ("net: phy: use phy_resolve_aneg_linkmode in genphy_read_status")
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>
> Tested-by: Andrew Lunn <andrew@lunn.ch>
>
> But i tested on net-next, not net.
>
You're right, it should be net-next. I just had the wrong automatism
fix = net in my mind.
> Andrew
>
>
Heiner
^ permalink raw reply
* Re: [PATCH v2 bpf-next 2/9] bpf: Add bpf helper bpf_tcp_enter_cwr
From: Eric Dumazet @ 2019-02-24 18:00 UTC (permalink / raw)
To: Martin Lau, Eric Dumazet
Cc: Lawrence Brakmo, netdev, Alexei Starovoitov, Daniel Borkmann,
Kernel Team
In-Reply-To: <20190224030845.imwjbkoaxipuzb75@kafai-mbp.dhcp.thefacebook.com>
On 02/23/2019 07:08 PM, Martin Lau wrote:
> On Sat, Feb 23, 2019 at 05:32:14PM -0800, Eric Dumazet wrote:
>>
>>
>> On 02/22/2019 05:06 PM, brakmo wrote:
>>> From: Martin KaFai Lau <kafai@fb.com>
>>>
>>> This patch adds a new bpf helper BPF_FUNC_tcp_enter_cwr
>>> "int bpf_tcp_enter_cwr(struct bpf_tcp_sock *tp)".
>>> It is added to BPF_PROG_TYPE_CGROUP_SKB which can be attached
>>> to the egress path where the bpf prog is called by
>>> ip_finish_output() or ip6_finish_output(). The verifier
>>> ensures that the parameter must be a tcp_sock.
>>>
>>> This helper makes a tcp_sock enter CWR state. It can be used
>>> by a bpf_prog to manage egress network bandwidth limit per
>>> cgroupv2. A later patch will have a sample program to
>>> show how it can be used to limit bandwidth usage per cgroupv2.
>>>
>>> To ensure it is only called from BPF_CGROUP_INET_EGRESS, the
>>> attr->expected_attach_type must be specified as BPF_CGROUP_INET_EGRESS
>>> during load time if the prog uses this new helper.
>>> The newly added prog->enforce_expected_attach_type bit will also be set
>>> if this new helper is used. This bit is for backward compatibility reason
>>> because currently prog->expected_attach_type has been ignored in
>>> BPF_PROG_TYPE_CGROUP_SKB. During attach time,
>>> prog->expected_attach_type is only enforced if the
>>> prog->enforce_expected_attach_type bit is set.
>>> i.e. prog->expected_attach_type is only enforced if this new helper
>>> is used by the prog.
>>>
>>
>> BTW, it seems to me that BPF_CGROUP_INET_EGRESS can be used while the socket lock is not held.
> Thanks for pointing it out.
>
> ic. I just noticed the comments at ip6_xmit():
> /*
> * xmit an sk_buff (used by TCP, SCTP and DCCP)
> * Note : socket lock is not held for SYNACK packets, but might be modified
> * by calls to skb_set_owner_w() and ipv6_local_error(),
> * which are using proper atomic operations or spinlocks.
> */
> Is there other cases other than SYNACK?
Well, I was referring to various virtual devices, re-entering ip stack.
Since we can have a qdisc on any netdev, there is no way we can guarantee the socket is
locked by the current thread.
Random example :
ipvlan_process_v4_outbound()
...
err = ip_local_out(net, skb->sk, skb);
...
^ permalink raw reply
* Re: [PATCH net] net: phy: fix reading fixed phy status
From: Andrew Lunn @ 2019-02-24 17:57 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org
In-Reply-To: <7ab8732d-bcc1-cc49-4bc8-34826e78038b@gmail.com>
On Sun, Feb 24, 2019 at 06:01:18PM +0100, Heiner Kallweit wrote:
> With the switch to phy_resolve_aneg_linkmode() we don't read from the
> chip any longer what is advertised but use phydev->advertising directly.
> For a fixed phy however this bitmap is empty so far, what results in
> no common mode being found. This breaks DSA. Fix this by advertising
> everything that is supported. For a normal phy this done by phy_probe().
>
> Fixes: 5502b218e001 ("net: phy: use phy_resolve_aneg_linkmode in genphy_read_status")
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Tested-by: Andrew Lunn <andrew@lunn.ch>
But i tested on net-next, not net.
Andrew
> ---
> drivers/net/phy/fixed_phy.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
> index b0d1368c3..1acd8bfdb 100644
> --- a/drivers/net/phy/fixed_phy.c
> +++ b/drivers/net/phy/fixed_phy.c
> @@ -22,6 +22,7 @@
> #include <linux/seqlock.h>
> #include <linux/idr.h>
> #include <linux/netdevice.h>
> +#include <linux/linkmode.h>
>
> #include "swphy.h"
>
> @@ -300,6 +301,8 @@ static struct phy_device *__fixed_phy_register(unsigned int irq,
> phy->supported);
> }
>
> + linkmode_copy(phy->advertising, phy->supported);
> +
> ret = phy_device_register(phy);
> if (ret) {
> phy_device_free(phy);
> --
> 2.20.1
>
^ permalink raw reply
* Re: [PATCH net-next] net: phy: improve auto-neg emulation in swphy
From: Andrew Lunn @ 2019-02-24 17:56 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Florian Fainelli, David Miller, Russell King - ARM Linux,
netdev@vger.kernel.org
In-Reply-To: <a290442f-fdae-26da-6073-3675bf635af1@gmail.com>
On Sun, Feb 24, 2019 at 05:41:47PM +0100, Heiner Kallweit wrote:
> Auto-neg emulation currently doesn't set bit BMCR_ANENABLE in BMCR,
> add this. Users will ignore speed and duplex settings in BMCR because
> we're emulating auto-neg, therefore we can remove related code.
> See also following discussion [0].
>
> [0] https://marc.info/?t=155041784900002&r=1&w=2
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Tested-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: No traffic with Marvell switch and latest linux-next
From: Andrew Lunn @ 2019-02-24 17:28 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Heiner Kallweit, Florian Fainelli, netdev@vger.kernel.org
In-Reply-To: <20190224153125.f2n3xfjyjswd7ioa@shell.armlinux.org.uk>
On Sun, Feb 24, 2019 at 03:31:26PM +0000, Russell King - ARM Linux admin wrote:
> On Sun, Feb 24, 2019 at 12:42:35AM +0100, Andrew Lunn wrote:
> > Looking forward, at some point we are going to have to make fixed-link
> > support higher speeds. That probably means we need a swphy-c45 which
> > emulates the standard registers for 2.5G, 5G and 10G. At that point
> > genphy will not work...
>
> Do we _need_ to emulate Clause 45 PHYs?
Hi Russell
One use case would be a mv88e6390X port 9 or 10 connected to a SoC
which can only do 2.5G.
We have defined that DSA drivers should configure CPU and DSA ports to
their maximum speed. So if port 9 or 10 is used, it should be
configured to 10G.
We then need some way to reconfigure the MAC to a slower speed.
We need to do this with ZII boards. We do this with:
port@0 {
reg = <0>;
label = "cpu";
ethernet = <&fec1>;
fixed-link {
speed = <100>;
full-duplex;
};
};
The FEC can only do 100Mbs, but the switch defaults to 1G. So the
fixed link it used to tell the switch MAC to use 100/Full.
In the example of the 6390X, we would want to set the link speed to
2500, which we cannot do at the moment. Either we need fixed-link to
support higher speeds, or we need a different mechanism.
We also have a similar issue on the SoC side. The FEC has no PHY
connected to it. It needs to be told what speed to do:
&fec1 {
phy-mode = "rmii";
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_fec1>;
status = "okay";
fixed-link {
speed = <100>;
full-duplex;
};
In the case of a SoC with an interface which can do 2.5G, you need to
tell it to do 2.5G. Ideally we want a mechanism that allows a MAC to
'see' a PHY operating at 2.5G using the standard phylib/phylink API.
In the past this has been achieved with an emulated PHY. But so long
as the phydev/phylink structure has the correct values, it does not
matter how they get those values.
Andrew
^ permalink raw reply
* Re: [PATCH iproute2-next v2 00/19] Export object IDs to users
From: Leon Romanovsky @ 2019-02-24 17:23 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, RDMA mailing list, Stephen Hemminger, Steve Wise
In-Reply-To: <c5995a79-e0d6-c88d-44d4-52d5477f2662@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 675 bytes --]
On Sun, Feb 24, 2019 at 10:16:37AM -0500, David Ahern wrote:
> On 2/23/19 4:15 AM, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@mellanox.com>
> >
> > Changelog:
> > v1->v2:
> > * Fixed commit messages
> > * Added Steve's ROB
> > v0->v1:
> > * Rebased to latest iproute2-next
> > * Added latest rdma_netlink.h and updated commit message to point
> > to kernel SHA commit.
> >
> > This series adds ability to present and query all known to rdmatool
> > object by their respective, unique IDs (e.g. pdn. mrn, cqn e.t.c).
> > All objects which have "parent" object has this information too.
> >
>
> applied to iproute2-next. Thanks, Leon.
Thanks, a lot.
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: No traffic with Marvell switch and latest linux-next
From: Andrew Lunn @ 2019-02-24 17:04 UTC (permalink / raw)
To: Florian Fainelli
Cc: Russell King - ARM Linux admin, Heiner Kallweit,
netdev@vger.kernel.org
In-Reply-To: <7005686d-45cb-a8af-de34-873c9e34a021@gmail.com>
> The added difficulty here and the reason why Andrew went with the
> approach that is used by the code currently is because neither do the
> CPU or DSA ports are backed by a net_device. It is somewhere on my TODO
> to permit the use of PHYLINK without the need of a net_device to cover
> those specific DSA cases unless we just brute force the whole thing and
> allocate a net_device structure but not register that net_device? Yes in
> fact, why don't we do that?
Hi Florian
At the moment, we are using a phydev which is not connected to a
MAC. That is rather odd, but the phylib maintainers mostly know about
this, and keep an eye out for changes which might break any
assumptions. And the phylib API is quite small.
How many assumptions are going to break with a netdev which is not
registered? The API is much bigger, more people hack on it, and it is
going to be much harder to review changes to make sure assumptions are
not changed.
If we are going to do something odd, we should keep the scope as small
as possible.
Andrew
^ permalink raw reply
* [PATCH net] net: phy: fix reading fixed phy status
From: Heiner Kallweit @ 2019-02-24 17:01 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
With the switch to phy_resolve_aneg_linkmode() we don't read from the
chip any longer what is advertised but use phydev->advertising directly.
For a fixed phy however this bitmap is empty so far, what results in
no common mode being found. This breaks DSA. Fix this by advertising
everything that is supported. For a normal phy this done by phy_probe().
Fixes: 5502b218e001 ("net: phy: use phy_resolve_aneg_linkmode in genphy_read_status")
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/fixed_phy.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
index b0d1368c3..1acd8bfdb 100644
--- a/drivers/net/phy/fixed_phy.c
+++ b/drivers/net/phy/fixed_phy.c
@@ -22,6 +22,7 @@
#include <linux/seqlock.h>
#include <linux/idr.h>
#include <linux/netdevice.h>
+#include <linux/linkmode.h>
#include "swphy.h"
@@ -300,6 +301,8 @@ static struct phy_device *__fixed_phy_register(unsigned int irq,
phy->supported);
}
+ linkmode_copy(phy->advertising, phy->supported);
+
ret = phy_device_register(phy);
if (ret) {
phy_device_free(phy);
--
2.20.1
^ permalink raw reply related
* Re: [PATCH ethtool] ethtool: Add support for 200Gbps (50Gbps per lane) link mode
From: Andrew Lunn @ 2019-02-24 16:47 UTC (permalink / raw)
To: Tariq Toukan; +Cc: John W. Linville, netdev, Eran Ben Elisha, Aya Levin
In-Reply-To: <1551020901-20257-1-git-send-email-tariqt@mellanox.com>
On Sun, Feb 24, 2019 at 05:08:21PM +0200, Tariq Toukan wrote:
> From: Aya Levin <ayal@mellanox.com>
> index 5a26cff5fb33..64ce0711ad5f 100644
> --- a/ethtool.8.in
> +++ b/ethtool.8.in
> @@ -650,6 +650,11 @@ lB l lB.
> 0x400000000 50000baseCR2 Full
> 0x800000000 50000baseKR2 Full
> 0x10000000000 50000baseSR2 Full
> +0x10000000000000 50000baseKR Full
> +0x20000000000000 50000baseSR Full
> +0x40000000000000 50000baseCR Full
> +0x80000000000000 50000baseLR_ER_FR Full
> +0x100000000000000 50000baseDR Full
> 0x8000000 56000baseKR4 Full
> 0x10000000 56000baseCR4 Full
> 0x20000000 56000baseSR4 Full
> @@ -658,6 +663,16 @@ lB l lB.
> 0x2000000000 100000baseSR4 Full
> 0x4000000000 100000baseCR4 Full
> 0x8000000000 100000baseLR4_ER4 Full
> +0x200000000000000 100000baseKR2 Full
> +0x400000000000000 100000baseSR2 Full
> +0x800000000000000 100000baseCR2 Full
> +0x1000000000000000 100000baseLR2_ER2_FR2 Full
> +0x2000000000000000 100000baseDR2 Full
> +0x4000000000000000 200000baseKR4 Full
> +0x8000000000000000 200000baseSR4 Full
> +0x10000000000000000 200000baseLR4_ER4_FR4 Full
> +0x20000000000000000 200000baseDR4 Full
> +0x40000000000000000 200000baseCR4 Full
This is getting less friendly all the time, and it was never very
friendly to start with. We have the strings which represent these link
modes in the table used for dumping caps. How about allowing the user
to list a comma separate list of modes.
ethtool -s lan42 advertise 100000baseKR2/Full,100000baseSR2/Full,100000baseCR2/Full
Andrew
> + adv_bit = ETHTOOL_LINK_MODE_200000baseKR4_Full_BIT &
> + ETHTOOL_LINK_MODE_200000baseSR4_Full_BIT &
> + ETHTOOL_LINK_MODE_200000baseLR4_ER4_FR4_Full_BIT &
> + ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT &
> + ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT;
Maybe i'm wrong, but this looks odd.
enum ethtool_link_mode_bit_indices {
ETHTOOL_LINK_MODE_10baseT_Half_BIT = 0,
ETHTOOL_LINK_MODE_10baseT_Full_BIT = 1,
ETHTOOL_LINK_MODE_100baseT_Half_BIT = 2,
ETHTOOL_LINK_MODE_100baseT_Full_BIT = 3,
ETHTOOL_LINK_MODE_1000baseT_Half_BIT = 4,
ETHTOOL_LINK_MODE_1000baseT_Full_BIT = 5,
These are numbers, not bitmasks, so & them together does not look
correct.
Andrew
^ permalink raw reply
* Re: [PATCH net-next 7/8] net: switchdev: Replace port attr set SDO with a notification
From: Florian Fainelli @ 2019-02-24 16:47 UTC (permalink / raw)
To: Ido Schimmel
Cc: netdev@vger.kernel.org, David S. Miller, open list,
open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE,
Jiri Pirko, andrew@lunn.ch, vivien.didelot@gmail.com
In-Reply-To: <20190223103207.GA13070@splinter>
Le 2/23/19 à 2:32 AM, Ido Schimmel a écrit :
> On Fri, Feb 22, 2019 at 03:59:25PM -0800, Florian Fainelli wrote:
>> Drop switchdev_ops.switchdev_port_attr_set. Drop the uses of this field
>> from all clients, which were migrated to use switchdev notification in
>> the previous patches.
>>
>> Add a new function switchdev_port_attr_notify() that sends the switchdev
>> notifications SWITCHDEV_PORT_ATTR_SET and takes care, depending on
>> SWITCHDEV_F_DEFER to call the blocking (process) or non-blocking
>> (atomic) notifier chain accordingly.
>>
>> Drop __switchdev_port_attr_set() and update switchdev_port_attr_set()
>> likewise.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> net/switchdev/switchdev.c | 96 +++++++++++----------------------------
>> 1 file changed, 26 insertions(+), 70 deletions(-)
>>
>> diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
>> index 94400f5b8e07..a1f16836ef89 100644
>> --- a/net/switchdev/switchdev.c
>> +++ b/net/switchdev/switchdev.c
>> @@ -174,81 +174,35 @@ static int switchdev_deferred_enqueue(struct net_device *dev,
>> return 0;
>> }
>>
>> -/**
>> - * switchdev_port_attr_get - Get port attribute
>
> Hmm, why do you remove it here? Can't you remove it in a separate patch?
> I thought we already got rid of it :p
Yes it should have been removed, looks like my previous series did not
that, I will send that separately.
>
>> - *
>> - * @dev: port device
>> - * @attr: attribute to get
>> - */
>> -int switchdev_port_attr_get(struct net_device *dev, struct switchdev_attr *attr)
>> +static int switchdev_port_attr_notify(enum switchdev_notifier_type nt,
>> + struct net_device *dev,
>> + const struct switchdev_attr *attr,
>> + struct switchdev_trans *trans)
>> {
>> - const struct switchdev_ops *ops = dev->switchdev_ops;
>> - struct net_device *lower_dev;
>> - struct list_head *iter;
>> - struct switchdev_attr first = {
>> - .id = SWITCHDEV_ATTR_ID_UNDEFINED
>> - };
>> - int err = -EOPNOTSUPP;
>> + int err;
>> + int rc;
>>
>> - if (ops && ops->switchdev_port_attr_get)
>> - return ops->switchdev_port_attr_get(dev, attr);
>> + struct switchdev_notifier_port_attr_info attr_info = {
>> + .attr = attr,
>> + .trans = trans,
>> + .handled = false,
>> + };
>>
>> - if (attr->flags & SWITCHDEV_F_NO_RECURSE)
>> + if (attr & SWITCHDEV_F_DEFER)
>> + rc = call_switchdev_blocking_notifiers(nt, dev,
>> + &attr_info.info, NULL);
>> + else
>> + rc = call_switchdev_notifiers(nt, dev, &attr_info.info, NULL);
>
> I don't believe this is needed. You're calling this function from
> switchdev_port_attr_set_now() which is always called from process
> context. switchdev_port_attr_set() takes care of that. Similar to
> switchdev_port_obj_add().
Except for net/bridge/br_switchdev.c when we check the bridge port's
flags support with PRE_BRIDGE_FLAGS. In that case we are executing from
the caller (atomic) context and we can't defer otherwise that trumps the
whole idea of being able to do a quick check and return that to the
caller that we cannot support specific flags. How would you recommend
approaching that?
--
Florian
^ permalink raw reply
* Re: [PATCH] Documentation: networking: switchdev: Update port parent ID section
From: Florian Fainelli @ 2019-02-24 16:46 UTC (permalink / raw)
To: David Miller; +Cc: netdev, andrew, vivien.didelot, idosch, jiri
In-Reply-To: <20190222.115839.1542447665920027412.davem@davemloft.net>
Le 2/22/19 à 11:58 AM, David Miller a écrit :
> From: Florian Fainelli <f.fainelli@gmail.com>
> Date: Wed, 20 Feb 2019 14:58:50 -0800
>
>> Update the section about switchdev drivers having to implement a
>> switchdev_port_attr_get() function to return
>> SWITCHDEV_ATTR_ID_PORT_PARENT_ID since that is no longer valid after
>> commit bccb30254a4a ("net: Get rid of
>> SWITCHDEV_ATTR_ID_PORT_PARENT_ID").
>>
>> Fixes: bccb30254a4a ("net: Get rid of SWITCHDEV_ATTR_ID_PORT_PARENT_ID")
>> Reviewed-by: Ido Schimmel <idosch@mellanox.com>
>> Acked-by: Jiri Pirko <jiri@mellanox.com>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>
> Applied.
>
Looks like you applied this to the "net" tree, and I missed prefixing
the patch to indicate this was targeted at the "net-next" tree. Are you
planning a merge from "net" to "net-next" anytime soon?
Thanks!
--
Florian
^ permalink raw reply
* [PATCH net-next] net: phy: improve auto-neg emulation in swphy
From: Heiner Kallweit @ 2019-02-24 16:41 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller,
Russell King - ARM Linux
Cc: netdev@vger.kernel.org
Auto-neg emulation currently doesn't set bit BMCR_ANENABLE in BMCR,
add this. Users will ignore speed and duplex settings in BMCR because
we're emulating auto-neg, therefore we can remove related code.
See also following discussion [0].
[0] https://marc.info/?t=155041784900002&r=1&w=2
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/swphy.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/net/phy/swphy.c b/drivers/net/phy/swphy.c
index b9743569e..dad22481d 100644
--- a/drivers/net/phy/swphy.c
+++ b/drivers/net/phy/swphy.c
@@ -19,7 +19,6 @@
#define MII_REGS_NUM 29
struct swmii_regs {
- u16 bmcr;
u16 bmsr;
u16 lpa;
u16 lpagb;
@@ -40,16 +39,13 @@ enum {
*/
static const struct swmii_regs speed[] = {
[SWMII_SPEED_10] = {
- .bmcr = BMCR_FULLDPLX,
.lpa = LPA_10FULL | LPA_10HALF,
},
[SWMII_SPEED_100] = {
- .bmcr = BMCR_FULLDPLX | BMCR_SPEED100,
.bmsr = BMSR_100FULL | BMSR_100HALF,
.lpa = LPA_100FULL | LPA_100HALF,
},
[SWMII_SPEED_1000] = {
- .bmcr = BMCR_FULLDPLX | BMCR_SPEED1000,
.bmsr = BMSR_ESTATEN,
.lpagb = LPA_1000FULL | LPA_1000HALF,
},
@@ -57,13 +53,11 @@ static const struct swmii_regs speed[] = {
static const struct swmii_regs duplex[] = {
[SWMII_DUPLEX_HALF] = {
- .bmcr = ~BMCR_FULLDPLX,
.bmsr = BMSR_ESTATEN | BMSR_100HALF,
.lpa = LPA_10HALF | LPA_100HALF,
.lpagb = LPA_1000HALF,
},
[SWMII_DUPLEX_FULL] = {
- .bmcr = ~0,
.bmsr = BMSR_ESTATEN | BMSR_100FULL,
.lpa = LPA_10FULL | LPA_100FULL,
.lpagb = LPA_1000FULL,
@@ -118,7 +112,6 @@ int swphy_read_reg(int reg, const struct fixed_phy_status *state)
{
int speed_index, duplex_index;
u16 bmsr = BMSR_ANEGCAPABLE;
- u16 bmcr = 0;
u16 lpagb = 0;
u16 lpa = 0;
@@ -136,7 +129,6 @@ int swphy_read_reg(int reg, const struct fixed_phy_status *state)
if (state->link) {
bmsr |= BMSR_LSTATUS | BMSR_ANEGCOMPLETE;
- bmcr |= speed[speed_index].bmcr & duplex[duplex_index].bmcr;
lpa |= speed[speed_index].lpa & duplex[duplex_index].lpa;
lpagb |= speed[speed_index].lpagb & duplex[duplex_index].lpagb;
@@ -149,7 +141,7 @@ int swphy_read_reg(int reg, const struct fixed_phy_status *state)
switch (reg) {
case MII_BMCR:
- return bmcr;
+ return BMCR_ANENABLE;
case MII_BMSR:
return bmsr;
case MII_PHYSID1:
--
2.20.1
Andr
^ permalink raw reply related
* Re: No traffic with Marvell switch and latest linux-next
From: Florian Fainelli @ 2019-02-24 16:32 UTC (permalink / raw)
To: Russell King - ARM Linux admin, Heiner Kallweit
Cc: Andrew Lunn, netdev@vger.kernel.org
In-Reply-To: <20190224154900.jmjuh4uz2lvtankg@shell.armlinux.org.uk>
Le 2/24/19 à 7:49 AM, Russell King - ARM Linux admin a écrit :
> On Sun, Feb 24, 2019 at 04:39:30PM +0100, Heiner Kallweit wrote:
>> On 24.02.2019 16:34, Russell King - ARM Linux admin wrote:
>>> On Sun, Feb 24, 2019 at 04:28:32PM +0100, Heiner Kallweit wrote:
>>>> On 24.02.2019 16:15, Russell King - ARM Linux admin wrote:
>>>>> On Sun, Feb 24, 2019 at 04:04:03PM +0100, Andrew Lunn wrote:
>>>>>>> I think what's not correct is that phydev->autoneg is set
>>>>>>> (by phy_device_create) for a fixed link.
>>>>>>
>>>>>> Fixed-link tries to emulate auto-neg:
>>>>>>
>>>>>> bmsr |= BMSR_LSTATUS | BMSR_ANEGCOMPLETE;
>>>>>>
>>>>>> Maybe it needs better emulation of auto-neg?
>>>>>
>>>>> Or maybe it needs to represent a fixed-speed PHY by clearing bit 1.3
>>>>> (BMSR_ANEGCAPABLE). In any case, 0.12 (BMCR_ANENABLE) is not set,
>>>>> so according to 802.3-2015, we should not be setting 1.5
>>>>> (BMSR_ANEGCOMPLETE).
>>>>>
>>>>> However, swphy does try to emulate autonegotiation - we do have cases
>>>>> where swphy is used in situations where the speed and duplex are not
>>>>> fixed. It returns an emulated link partner advertisement for the
>>>>> current speed, which would suggest that we should set BMCR_ANENABLE.
>>>>>
>>>> If we emulate auto-neg, then it's not needed to set the speed bits
>>>> in BMCR. Also what just comes to my mind, certain speeds like 1000BaseT
>>>> don't support forced mode. So we may have to go with auto-neg.
>>>
>>> Sure.
>>>
>>>> To avoid the original issue it should be sufficient to copy
>>>> supported -> advertising at a suited place.
>>>
>> Sorry, seems this wasn't clear enough. I don't mean to change
>> swphy but the user side.
>>
>>> Why bother - the software PHY emulation is an emulation to allow
>>> existing userspace that pre-dates the ethtool API to get some link
>>> parameters. If we augment the PHY emulation in non-standard ways,
>>> userspace will need to be updated to handle those non-standard
>>> ways. If userspace needs to be updated, why not just bite the
>>> bullet and update to ethtool APIs rather than adding more
>>> complication through an emulation layer?
>>>
>> It's not only userspace. Based on my limited knowledge of DSA this
>> code also uses e.g. genphy_read_status() with a fixed link.
>
> DSA has support for phylink, which is perfectly capable of supporting
> fixed links without using fixed-phy.c, although we have no way to
> create that without DT. Support could be added for non-DT though.
I had a branch at some point that provided feature parity with what DT
registration is capable of doing, including representing links between
switches etc [1]. I am not entirely sure it makes to support such a
configuration for platform devices, since they are not so many these
days that would not use DT (except x86 maybe?). It would make sense to
support ACPI-based registration for PHYLINK, if there was a standard for
describing PHYs, SFP/SFFs which is not apparently the case.
[1]: https://github.com/ffainelli/linux/commits/dsa-lbk-pdata2
>
> That would avoid using phylib functions to read back from a fixed-link
> PHY. Since phylink presents to the MAC a fixed link in the same
> abstract manner as a real PHY, it should result in a more elegant
> implementation.
>
> DSA already has phylink support to support SFPs.
>
The added difficulty here and the reason why Andrew went with the
approach that is used by the code currently is because neither do the
CPU or DSA ports are backed by a net_device. It is somewhere on my TODO
to permit the use of PHYLINK without the need of a net_device to cover
those specific DSA cases unless we just brute force the whole thing and
allocate a net_device structure but not register that net_device? Yes in
fact, why don't we do that?
--
Florian
^ permalink raw reply
* Re: [PATCH 0/3] Cover letter: Add support for high speed MAC in Cadence controller driver
From: Florian Fainelli @ 2019-02-24 16:22 UTC (permalink / raw)
To: Parshuram Thombare, nicolas.ferre, davem, netdev, andrew,
hkallweit1, linux-kernel, rafalc, piotrs, jank
In-Reply-To: <20190222201118.GA14791@lvlogina.cadence.com>
Hi,
Le 2/22/19 à 12:11 PM, Parshuram Thombare a écrit :
> Hello !
>
> This patch series contain changes to support high speed MAC and PCS
> in Cadence ethernet controller driver.
From patch submission perspective, your cover letter and individual
patches do not appear as a reply to this cover letter which makes the
threading broken in most mail clients. If you used git format-patch
--cover-letter, this is handled transparently for you by git send-email.
>
> Here is the list of patches.
> [PATCH 1/3] net: ethernet: add support for PCS and 2.5G speed
> This patch add support for PCS (for SGMII interface) and 2.5Gbps
> MAC in Cadence ethernet controller driver.
> [PATCH 2/3] net: ethernet: add c45 PHY support in MDIO read/write functions.
> This patch modify MDIO read/write functions to support
> communication with C45 PHY in Cadence ethernet controller driver.
> [PATCH 3/3] net: ethernet: add support for high speed mac and usxgmii pcs
> This patch add support for (10G) high speed mac, usxgmii pcs
> and usxgmii interface in Cadence ethernet controller driver.
>
> Regards,
> Parshuram Thombare
>
> Parshuram Thombare (3):
> net: ethernet: add support for PCS and 2.5G speed
> net: ethernet: add c45 PHY support in MDIO read/write functions.
> net: ethernet: add support for high speed mac and usxgmii pcs
>
> drivers/net/ethernet/cadence/macb.h | 68 ++++++-
> drivers/net/ethernet/cadence/macb_main.c | 309 +++++++++++++++++++++++++----
> include/linux/phy.h | 3 +
> 3 files changed, 333 insertions(+), 47 deletions(-)
>
--
Florian
^ permalink raw reply
* Re: [PATCH net] staging: fsl-dpaa2: ethsw: Add missing netdevice check
From: Florian Fainelli @ 2019-02-24 16:17 UTC (permalink / raw)
To: Ioana Ciornei, netdev@vger.kernel.org
Cc: Ioana Ciocoi Radulescu, Greg Kroah-Hartman,
open list:DPAA2 ETHERNET SWITCH DRIVER,
open list:STAGING SUBSYSTEM
In-Reply-To: <VI1PR0402MB28009A8333F247A753F44F94E0780@VI1PR0402MB2800.eurprd04.prod.outlook.com>
Le 2/23/19 à 12:45 AM, Ioana Ciornei a écrit :
>
>> Subject: [PATCH net] staging: fsl-dpaa2: ethsw: Add missing netdevice check
>>
>> port_switchdev_event() does not check that the target network device is
>> actually backed by the ethsw driver, this could be problematic in a stacked
>> environment case.
>>
>
> Just FYI, I sent a patch set containing a similar patch verifying if the netdev is backed by the ethsw:
> https://lkml.org/lkml/2019/2/6/216
>
> I sent the entire patch set against the staging tree.
Thank you. BTW do you have any plans for moving this driver out of
staging? It looks pretty good to me to get promoted in tree, provided
that you also have all the dependencies in place etc.
--
Florian
^ permalink raw reply
* Re: No traffic with Marvell switch and latest linux-next
From: Russell King - ARM Linux admin @ 2019-02-24 15:49 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: Andrew Lunn, Florian Fainelli, netdev@vger.kernel.org
In-Reply-To: <bdca070b-53ea-72ab-4864-e5d36048553f@gmail.com>
On Sun, Feb 24, 2019 at 04:39:30PM +0100, Heiner Kallweit wrote:
> On 24.02.2019 16:34, Russell King - ARM Linux admin wrote:
> > On Sun, Feb 24, 2019 at 04:28:32PM +0100, Heiner Kallweit wrote:
> >> On 24.02.2019 16:15, Russell King - ARM Linux admin wrote:
> >>> On Sun, Feb 24, 2019 at 04:04:03PM +0100, Andrew Lunn wrote:
> >>>>> I think what's not correct is that phydev->autoneg is set
> >>>>> (by phy_device_create) for a fixed link.
> >>>>
> >>>> Fixed-link tries to emulate auto-neg:
> >>>>
> >>>> bmsr |= BMSR_LSTATUS | BMSR_ANEGCOMPLETE;
> >>>>
> >>>> Maybe it needs better emulation of auto-neg?
> >>>
> >>> Or maybe it needs to represent a fixed-speed PHY by clearing bit 1.3
> >>> (BMSR_ANEGCAPABLE). In any case, 0.12 (BMCR_ANENABLE) is not set,
> >>> so according to 802.3-2015, we should not be setting 1.5
> >>> (BMSR_ANEGCOMPLETE).
> >>>
> >>> However, swphy does try to emulate autonegotiation - we do have cases
> >>> where swphy is used in situations where the speed and duplex are not
> >>> fixed. It returns an emulated link partner advertisement for the
> >>> current speed, which would suggest that we should set BMCR_ANENABLE.
> >>>
> >> If we emulate auto-neg, then it's not needed to set the speed bits
> >> in BMCR. Also what just comes to my mind, certain speeds like 1000BaseT
> >> don't support forced mode. So we may have to go with auto-neg.
> >
> > Sure.
> >
> >> To avoid the original issue it should be sufficient to copy
> >> supported -> advertising at a suited place.
> >
> Sorry, seems this wasn't clear enough. I don't mean to change
> swphy but the user side.
>
> > Why bother - the software PHY emulation is an emulation to allow
> > existing userspace that pre-dates the ethtool API to get some link
> > parameters. If we augment the PHY emulation in non-standard ways,
> > userspace will need to be updated to handle those non-standard
> > ways. If userspace needs to be updated, why not just bite the
> > bullet and update to ethtool APIs rather than adding more
> > complication through an emulation layer?
> >
> It's not only userspace. Based on my limited knowledge of DSA this
> code also uses e.g. genphy_read_status() with a fixed link.
DSA has support for phylink, which is perfectly capable of supporting
fixed links without using fixed-phy.c, although we have no way to
create that without DT. Support could be added for non-DT though.
That would avoid using phylib functions to read back from a fixed-link
PHY. Since phylink presents to the MAC a fixed link in the same
abstract manner as a real PHY, it should result in a more elegant
implementation.
DSA already has phylink support to support SFPs.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* Re: No traffic with Marvell switch and latest linux-next
From: Heiner Kallweit @ 2019-02-24 15:39 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Andrew Lunn, Florian Fainelli, netdev@vger.kernel.org
In-Reply-To: <20190224153449.gg3c3fyryztdj3vm@shell.armlinux.org.uk>
On 24.02.2019 16:34, Russell King - ARM Linux admin wrote:
> On Sun, Feb 24, 2019 at 04:28:32PM +0100, Heiner Kallweit wrote:
>> On 24.02.2019 16:15, Russell King - ARM Linux admin wrote:
>>> On Sun, Feb 24, 2019 at 04:04:03PM +0100, Andrew Lunn wrote:
>>>>> I think what's not correct is that phydev->autoneg is set
>>>>> (by phy_device_create) for a fixed link.
>>>>
>>>> Fixed-link tries to emulate auto-neg:
>>>>
>>>> bmsr |= BMSR_LSTATUS | BMSR_ANEGCOMPLETE;
>>>>
>>>> Maybe it needs better emulation of auto-neg?
>>>
>>> Or maybe it needs to represent a fixed-speed PHY by clearing bit 1.3
>>> (BMSR_ANEGCAPABLE). In any case, 0.12 (BMCR_ANENABLE) is not set,
>>> so according to 802.3-2015, we should not be setting 1.5
>>> (BMSR_ANEGCOMPLETE).
>>>
>>> However, swphy does try to emulate autonegotiation - we do have cases
>>> where swphy is used in situations where the speed and duplex are not
>>> fixed. It returns an emulated link partner advertisement for the
>>> current speed, which would suggest that we should set BMCR_ANENABLE.
>>>
>> If we emulate auto-neg, then it's not needed to set the speed bits
>> in BMCR. Also what just comes to my mind, certain speeds like 1000BaseT
>> don't support forced mode. So we may have to go with auto-neg.
>
> Sure.
>
>> To avoid the original issue it should be sufficient to copy
>> supported -> advertising at a suited place.
>
Sorry, seems this wasn't clear enough. I don't mean to change
swphy but the user side.
> Why bother - the software PHY emulation is an emulation to allow
> existing userspace that pre-dates the ethtool API to get some link
> parameters. If we augment the PHY emulation in non-standard ways,
> userspace will need to be updated to handle those non-standard
> ways. If userspace needs to be updated, why not just bite the
> bullet and update to ethtool APIs rather than adding more
> complication through an emulation layer?
>
It's not only userspace. Based on my limited knowledge of DSA this
code also uses e.g. genphy_read_status() with a fixed link.
^ permalink raw reply
* Re: No traffic with Marvell switch and latest linux-next
From: Russell King - ARM Linux admin @ 2019-02-24 15:34 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: Andrew Lunn, Florian Fainelli, netdev@vger.kernel.org
In-Reply-To: <403607a0-16d8-15b6-f0aa-b8b13793d401@gmail.com>
On Sun, Feb 24, 2019 at 04:28:32PM +0100, Heiner Kallweit wrote:
> On 24.02.2019 16:15, Russell King - ARM Linux admin wrote:
> > On Sun, Feb 24, 2019 at 04:04:03PM +0100, Andrew Lunn wrote:
> >>> I think what's not correct is that phydev->autoneg is set
> >>> (by phy_device_create) for a fixed link.
> >>
> >> Fixed-link tries to emulate auto-neg:
> >>
> >> bmsr |= BMSR_LSTATUS | BMSR_ANEGCOMPLETE;
> >>
> >> Maybe it needs better emulation of auto-neg?
> >
> > Or maybe it needs to represent a fixed-speed PHY by clearing bit 1.3
> > (BMSR_ANEGCAPABLE). In any case, 0.12 (BMCR_ANENABLE) is not set,
> > so according to 802.3-2015, we should not be setting 1.5
> > (BMSR_ANEGCOMPLETE).
> >
> > However, swphy does try to emulate autonegotiation - we do have cases
> > where swphy is used in situations where the speed and duplex are not
> > fixed. It returns an emulated link partner advertisement for the
> > current speed, which would suggest that we should set BMCR_ANENABLE.
> >
> If we emulate auto-neg, then it's not needed to set the speed bits
> in BMCR. Also what just comes to my mind, certain speeds like 1000BaseT
> don't support forced mode. So we may have to go with auto-neg.
Sure.
> To avoid the original issue it should be sufficient to copy
> supported -> advertising at a suited place.
Why bother - the software PHY emulation is an emulation to allow
existing userspace that pre-dates the ethtool API to get some link
parameters. If we augment the PHY emulation in non-standard ways,
userspace will need to be updated to handle those non-standard
ways. If userspace needs to be updated, why not just bite the
bullet and update to ethtool APIs rather than adding more
complication through an emulation layer?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox