* Re: [PATCH] net: do not allow changing SO_REUSEADDR/SO_REUSEPORT on bound sockets
From: David Miller @ 2018-06-04 21:14 UTC (permalink / raw)
To: zenczykowski; +Cc: maze, edumazet, netdev
In-Reply-To: <20180603174705.51802-1-zenczykowski@gmail.com>
From: "Maciej Żenczykowski" <zenczykowski@gmail.com>
Date: Sun, 3 Jun 2018 10:47:05 -0700
> From: Maciej Żenczykowski <maze@google.com>
>
> It is not safe to do so because such sockets are already in the
> hash tables and changing these options can result in invalidating
> the tb->fastreuse(port) caching.
>
> This can have later far reaching consequences wrt. bind conflict checks
> which rely on these caches (for optimization purposes).
>
> Not to mention that you can currently end up with two identical
> non-reuseport listening sockets bound to the same local ip:port
> by clearing reuseport on them after they've already both been bound.
>
> There is unfortunately no EISBOUND error or anything similar,
> and EISCONN seems to be misleading for a bound-but-not-connected
> socket, so use EUCLEAN 'Structure needs cleaning' which AFAICT
> is the closest you can get to meaning 'socket in bad state'.
> (although perhaps EINVAL wouldn't be a bad choice either?)
>
> This does unfortunately run the risk of breaking buggy
> userspace programs...
>
> Signed-off-by: Maciej Żenczykowski <maze@google.com>
> Change-Id: I77c2b3429b2fdf42671eee0fa7a8ba721c94963b
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net-next] net: ipv6: Generate random IID for addresses on RAWIP devices
From: David Miller @ 2018-06-04 21:15 UTC (permalink / raw)
To: subashab; +Cc: netdev
In-Reply-To: <1528062874-19250-1-git-send-email-subashab@codeaurora.org>
From: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
Date: Sun, 3 Jun 2018 15:54:34 -0600
> RAWIP devices such as rmnet do not have a hardware address and
> instead require the kernel to generate a random IID for the
> temporary addresses. For permanent addresses, the device IID is
> used along with prefix received.
>
> Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
Please address yoshfuji's feedback, thank you.
^ permalink raw reply
* Re: [PATCH iproute2 1/2] ip: display netns name instead of nsid
From: Stephen Hemminger @ 2018-06-04 21:12 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: netdev
In-Reply-To: <20180604121253.2140-2-nicolas.dichtel@6wind.com>
On Mon, 4 Jun 2018 14:12:52 +0200
Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
> index c7c7e7df4e81..aee09c7ff6df 100644
> --- a/ip/ipaddress.c
> +++ b/ip/ipaddress.c
> @@ -819,6 +819,9 @@ int print_linkinfo(const struct sockaddr_nl *who,
> unsigned int m_flag = 0;
> SPRINT_BUF(b1);
>
> + netns_nsid_socket_init();
> + netns_map_init();
> +
The idea of printing network namespace is good but I am concerned that
setting up yet another netlink socket and scanning the netns directory on
each ip link command will impact some users.
Can this setup be deferred until the first net ns lookup happens?
^ permalink raw reply
* Re: [PATCH net] net: qualcomm: rmnet: Fix use after free while sending command ack
From: David Miller @ 2018-06-04 21:16 UTC (permalink / raw)
To: subashab; +Cc: netdev
In-Reply-To: <1528064275-3205-1-git-send-email-subashab@codeaurora.org>
From: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
Date: Sun, 3 Jun 2018 16:17:55 -0600
> When sending an ack to a command packet, the skb is still referenced
> after it is sent to the real device. Since the real device could
> free the skb, the device pointer would be invalid.
>
> Fixes: ceed73a2cf4a ("drivers: net: ethernet: qualcomm: rmnet: Initial implementation")
> Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
This doesn't apply cleanly to the current net-next tree, please respin.
^ permalink raw reply
* Re: [PATCH] docs: networking: fix minor typos in various documentation files
From: David Miller @ 2018-06-04 21:21 UTC (permalink / raw)
To: olivier.gayot; +Cc: netdev
In-Reply-To: <20180604100737.4228-1-olivier.gayot@sigexec.com>
From: Olivier Gayot <olivier.gayot@sigexec.com>
Date: Mon, 4 Jun 2018 12:07:37 +0200
> This patch fixes some typos/misspelling errors in the
> Documentation/networking files.
>
> Signed-off-by: Olivier Gayot <olivier.gayot@sigexec.com>
Applied, thank you.
^ permalink raw reply
* Re: pull request: bluetooth-next 2018-06-04
From: David Miller @ 2018-06-04 21:22 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth, netdev
In-Reply-To: <20180604101532.GA8919@x1c>
From: Johan Hedberg <johan.hedberg@gmail.com>
Date: Mon, 4 Jun 2018 12:15:32 +0200
> Here's one last bluetooth-next pull request for the 4.18 kernel:
>
> - New USB device IDs for Realtek 8822BE and 8723DE
> - reset/resume fix for Dell Inspiron 5565
> - Fix HCI_UART_INIT_PENDING flag behavior
> - Fix patching behavior for some ATH3012 models
> - A few other minor cleanups & fixes
>
> Please let me know if there are any issues pulling. Thanks.
Pulled, thanks Johan.
^ permalink raw reply
* Re: [PATCH net] ipmr: fix error path when mr_table_alloc fails
From: David Miller @ 2018-06-04 21:25 UTC (permalink / raw)
To: sd; +Cc: netdev, edumazet, nikolay, yuvalm, ivecera
In-Reply-To: <ffc3366697c8d8789cbe314b6944b910eceb38e7.1528112758.git.sd@queasysnail.net>
From: Sabrina Dubroca <sd@queasysnail.net>
Date: Mon, 4 Jun 2018 13:55:54 +0200
> commit 0bbbf0e7d0e7 ("ipmr, ip6mr: Unite creation of new mr_table")
> refactored ipmr_new_table, so that it now returns NULL when
> mr_table_alloc fails. Unfortunately, all callers of ipmr_new_table
> expect an ERR_PTR. commit 66fb33254f45 ("ipmr: properly check
> rhltable_init() return value") followed suit.
>
> This can result in NULL deref, when ipmr_rules_exit calls
> ipmr_free_table with NULL net->ipv4.mrt in the
> !CONFIG_IP_MROUTE_MULTIPLE_TABLES version.
>
> This patch makes mr_table_alloc return errors, and changes
> ip6mr_new_table and its callers to return/expect error pointers as
> well. It also removes the version of mr_table_alloc defined under
> !CONFIG_IP_MROUTE_COMMON, since it is never used.
>
> Fixes: 0bbbf0e7d0e7 ("ipmr, ip6mr: Unite creation of new mr_table")
> Fixes: 66fb33254f45 ("ipmr: properly check rhltable_init() return value")
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
This adds a new warning with gcc-8.1.1 on Fedora 28
CC [M] net/ipv6/ip6mr.o
In file included from ./arch/x86/include/asm/current.h:5,
from ./include/linux/sched.h:12,
from ./include/linux/uaccess.h:5,
from net/ipv6/ip6mr.c:19:
net/ipv6/ip6mr.c: In function ‘ip6_mroute_setsockopt’:
./include/linux/compiler.h:177:26: warning: ‘mrt’ may be used uninitialized in this function [-Wmaybe-uninitialized]
case 8: *(__u64 *)res = *(volatile __u64 *)p; break; \
^
net/ipv6/ip6mr.c:1752:20: note: ‘mrt’ was declared here
struct mr_table *mrt;
^~~
^ permalink raw reply
* Re: [net-next 00/12][pull request] Intel Wired LAN Driver Updates 2018-06-04
From: David Miller @ 2018-06-04 21:27 UTC (permalink / raw)
To: gerlitz.or; +Cc: jeffrey.t.kirsher, netdev
In-Reply-To: <CAJ3xEMg1cSodYJwXLyoUkdwDJicKoeG67Y_vnfFfPQU====vZA@mail.gmail.com>
From: Or Gerlitz <gerlitz.or@gmail.com>
Date: Tue, 5 Jun 2018 00:11:35 +0300
> Just to make sure, is the AF_XDP ZC (Zero Copy) UAPI going to be
> merged for this window -- AFAIU from [1], it's still under
> examination/development/research for non Intel HWs, am I correct or
> this is going to get in now?
All of the pending AF_XDP changes will be merged this merge window.
I think Intel folks need to review things as fast as possible because
I pretty much refuse to revert the series or disable it in Kconfig at
this point.
Thank you.
^ permalink raw reply
* Re: [PATCH net-next] wan/fsl_ucc_hdlc: use dma_zalloc_coherent instead of allocator/memset
From: David Miller @ 2018-06-04 21:28 UTC (permalink / raw)
To: yuehaibing; +Cc: qiang.zhao, netdev, linux-kernel, linuxppc-dev
In-Reply-To: <20180604130759.25024-1-yuehaibing@huawei.com>
From: YueHaibing <yuehaibing@huawei.com>
Date: Mon, 4 Jun 2018 21:07:59 +0800
> Use dma_zalloc_coherent instead of dma_alloc_coherent
> followed by memset 0.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] qed: use dma_zalloc_coherent instead of allocator/memset
From: David Miller @ 2018-06-04 21:28 UTC (permalink / raw)
To: yuehaibing; +Cc: Ariel.Elior, netdev, linux-kernel, everest-linux-l2
In-Reply-To: <20180604131031.24476-1-yuehaibing@huawei.com>
From: YueHaibing <yuehaibing@huawei.com>
Date: Mon, 4 Jun 2018 21:10:31 +0800
> Use dma_zalloc_coherent instead of dma_alloc_coherent
> followed by memset 0.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied.
^ permalink raw reply
* Re: [PATCH 1/2 v2 net-next] net_failover: Use netdev_features_t instead of u32
From: David Miller @ 2018-06-04 21:30 UTC (permalink / raw)
To: dan.carpenter; +Cc: sridhar.samudrala, netdev, kernel-janitors
In-Reply-To: <20180604144321.kjemxdfhi4qqb5ji@kili.mountain>
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Mon, 4 Jun 2018 17:43:21 +0300
> The features mask needs to be a netdev_features_t (u64) because a u32
> is not big enough.
>
> Fixes: cfc80d9a1163 ("net: Introduce net_failover driver")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: In the original patch, I thought that the & should be | and I
> introduced a bug.
Applied.
^ permalink raw reply
* Re: [PATCH 10/18] rhashtable: remove rhashtable_walk_peek()
From: Tom Herbert @ 2018-06-04 21:31 UTC (permalink / raw)
To: NeilBrown
Cc: Herbert Xu, Thomas Graf, Linux Kernel Network Developers, LKML,
Tom Herbert
In-Reply-To: <87sh63pakb.fsf@notabene.neil.brown.name>
On Sun, Jun 3, 2018 at 7:09 PM, NeilBrown <neilb@suse.com> wrote:
> On Sun, Jun 03 2018, Tom Herbert wrote:
>
>> On Sun, Jun 3, 2018 at 5:30 PM, NeilBrown <neilb@suse.com> wrote:
>>> On Sat, Jun 02 2018, Herbert Xu wrote:
>>>
>>>> On Fri, Jun 01, 2018 at 02:44:09PM +1000, NeilBrown wrote:
>>>>> This function has a somewhat confused behavior that is not properly
>>>>> described by the documentation.
>>>>> Sometimes is returns the previous object, sometimes it returns the
>>>>> next one.
>>>>> Sometimes it changes the iterator, sometimes it doesn't.
>>>>>
>>>>> This function is not currently used and is not worth keeping, so
>>>>> remove it.
>>>>>
>>>>> A future patch will introduce a new function with a
>>>>> simpler interface which can meet the same need that
>>>>> this was added for.
>>>>>
>>>>> Signed-off-by: NeilBrown <neilb@suse.com>
>>>>
>>>> Please keep Tom Herbert in the loop. IIRC he had an issue with
>>>> this patch.
>>>
>>> Yes you are right - sorry for forgetting to add Tom.
>>>
>>> My understanding of where this issue stands is that Tom raised issue and
>>> asked for clarification, I replied, nothing further happened.
>>>
>>> It summary, my position is that:
>>> - most users of my new rhashtable_walk_prev() will use it like
>>> rhasthable_talk_prev() ?: rhashtable_walk_next()
>>> which is close to what rhashtable_walk_peek() does
>>> - I know of no use-case that could not be solved if we only had
>>> the combined operation
>>> - BUT it is hard to document the combined operation, as it really
>>> does two things. If it is hard to document, then it might be
>>> hard to understand.
>>>
>>> So provide the most understandable/maintainable solution, I think
>>> we should provide rhashtable_walk_prev() as a separate interface.
>>>
>> I'm still missing why requiring two API operations instead of one is
>> simpler or easier to document. Also, I disagree that
>> rhashtable_walk_peek does two things-- it just does one which is to
>> return the current element in the walk without advancing to the next
>> one. The fact that the iterator may or may not move is immaterial in
>> the API, that is an implementation detail. In fact, it's conceivable
>> that we might completely reimplement this someday such that the
>> iterator works completely differently implementation semantics but the
>> API doesn't change. Also the naming in your proposal is confusing,
>> we'd have operations to get the previous, and the next next object--
>> so the user may ask where's the API to get the current object in the
>> walk? The idea that we get it by first trying to get the previous
>> object, and then if that fails getting the next object seems
>> counterintuitive.
>
> To respond to your points out of order:
>
> - I accept that "rhashtable_walk_prev" is not a perfect name. It
> suggests a stronger symmetry with rhasthable_walk_next than actually
> exist. I cannot think of a better name, but I think the
> description "Return the previously returned object if it is
> still in the table" is clear and simple and explains the name.
> I'm certainly open to suggestions for a better name.
>
> - I don't think it is meaningful to talk about a "current" element in a
> table where asynchronous insert/remove is to be expected.
> The best we can hope for is a "current location" is the sequence of
> objects in the table - a location which is after some objects and
> before all others. rhashtable_walk_next() returns the next object
> after the current location, and advances the location pointer past
> that object.
> rhashtable_walk_prev() *doesn't* return the previous object in the
> table. It returns the previously returned object. ("previous" in
> time, but not in space, if you like).
>
> - rhashtable_walk_peek() currently does one of two different things.
> It either returns the previously returned object (iter->p) if that
> is still in the table, or it find the next object, steps over it, and
> returns it.
>
> - I would like to suggest that when an API acts on a iterator object,
> the question of whether or not the iterator is advanced *must* be a
> fundamental question, not one that might change from time to time.
>
> Maybe a useful way forward would be for you to write documentation for
> the rhashtable_walk_peek() interface which correctly describes what it
> does and how it is used. Given that, I can implement that interface
> with the stability improvements that I'm working on.
>
Here's how it's documented currently:
"rhashtable_walk_peek - Return the next object but don't advance the iterator"
I don't see what is incorrect about that. Peek returns the next object
in the walk, however does not move the iterator past that object, so
sucessive calls to peek return the same object. In other words it's a
way to inspect the next object but not "consume" it. This is what is
needed when netlink returns in the middle of a walk. The last object
retrieved from the table may not have been processed completely, so it
needs to be the first one processed on the next invocation to netlink.
This is also easily distinguishable from
"rhashtable_walk_next - Return the next object and advance the iterator"
Where the only difference is that peek and walk is that, walk advances
the iterator and peek does not. Hence why "peek" is a descriptive name
for what is happening.
Tom
> Thanks,
> NeilBrown
^ permalink raw reply
* Re: [PATCH 2/2 net] team: use netdev_features_t instead of u32
From: David Miller @ 2018-06-04 21:31 UTC (permalink / raw)
To: dan.carpenter; +Cc: jiri, netdev, kernel-janitors
In-Reply-To: <20180604144601.wo5iqb5irfes4frv@kili.mountain>
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Mon, 4 Jun 2018 17:46:01 +0300
> This code was introduced in 2011 around the same time that we made
> netdev_features_t a u64 type. These days a u32 is not big enough to
> hold all the potential features.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net-next] net: sched: return error code when tcf proto is not found
From: David Miller @ 2018-06-04 21:32 UTC (permalink / raw)
To: vladbu; +Cc: netdev, jhs, xiyou.wangcong, jiri, dan.carpenter
In-Reply-To: <1528126343-13104-1-git-send-email-vladbu@mellanox.com>
From: Vlad Buslov <vladbu@mellanox.com>
Date: Mon, 4 Jun 2018 18:32:23 +0300
> If requested tcf proto is not found, get and del filter netlink protocol
> handlers output error message to extack, but do not return actual error
> code. Add check to return ENOENT when result of tp find function is NULL
> pointer.
>
> Fixes: c431f89b18a2 ("net: sched: split tc_ctl_tfilter into three
> handlers")
Please do not split up a Fixes: tag into multiple lines. I fixed it
up for you this time.
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Applied, thanks.
^ permalink raw reply
* issues with shaping cake to 60 gbits or in unlimited mode at 100gbps
From: Dave Taht @ 2018-06-04 21:32 UTC (permalink / raw)
To: David Miller
Cc: Toke Høiland-Jørgensen, Linux Kernel Network Developers,
Cake List, George Amanakis, Pete Heist, Yuchung Cheng,
Neal Cardwell, Netfilter Development Mailinglist
I would like to point a finger at the mlx5 code rather than cake itself,
I am wondering if there is anyone out there with a different 100gps
capable device and setup that could give the sch_cake code a go?
we've been unable to duplicate the lockup we're seeing with the veth devices,
for example.
On Fri, Jun 1, 2018 at 11:17 AM, David Miller <davem@davemloft.net> wrote:
> From: Toke Høiland-Jørgensen <toke@toke.dk>
> Date: Fri, 01 Jun 2018 19:42:12 +0200
>
>> Toke Høiland-Jørgensen <toke@toke.dk> writes:
>>
>>> This patch series adds the CAKE qdisc, and has been split up to ease
>>> review.
>>>
>>> I have attempted to split out each configurable feature into its own patch.
>>> The first commit adds the base shaper and packet scheduler, while
>>> subsequent commits add the optional features. The full userspace API and
>>> most data structures are included in this commit, but options not
>>> understood in the base version will be ignored.
>>
>> Hmm, there seems to be a lockup issue being triggered when running CAKE
>> at high speeds (>40 Gbps in my tests). Please drop this series for now,
>> we'll return for 4.19 with a revised version.
>
> Ok.
>
> Thanks for continuing to push this work along.
--
Dave Täht
CEO, TekLibre, LLC
http://www.teklibre.com
Tel: 1-669-226-2619
^ permalink raw reply
* Re: [PATCH V2 net] net: hns: Fix the process of adding broadcast addresses to tcam
From: David Miller @ 2018-06-04 21:33 UTC (permalink / raw)
To: salil.mehta
Cc: yisen.zhuang, lipeng321, mehta.salil, netdev, linux-kernel,
linuxarm, wangxi11
In-Reply-To: <20180604165009.24476-1-salil.mehta@huawei.com>
From: Salil Mehta <salil.mehta@huawei.com>
Date: Mon, 4 Jun 2018 17:50:09 +0100
> From: Xi Wang <wangxi11@huawei.com>
>
> If the multicast mask value in device tree is configured not all
> 0xff, the broadcast mac will be lost from tcam table after the
> execution of command 'ifconfig up'. The address is appended by
> hns_ae_start, but will be clear later by hns_nic_set_rx_mode
> called in dev_open process.
>
> This patch fixed it by not use the multicast mask when add a
> broadcast address.
>
> Fixes: b5996f11ea54 ("net: add Hisilicon Network Subsystem basic ethernet support")
> Signed-off-by: Xi Wang <wangxi11@huawei.com>
> Signed-off-by: Peng Li <lipeng321@huawei.com>
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> ---
> Patch V2: Fixed comments by David Miller
> Link: https://lkml.org/lkml/2018/6/4/671
> Patch V1: Initial Submit
Applied and queued up for -stable, thank you.
^ permalink raw reply
* Re: [PATCH 15a/18] rhashtables: add lockdep tracking to bucket bit-spin-locks.
From: NeilBrown @ 2018-06-04 21:37 UTC (permalink / raw)
To: Simon Horman
Cc: Eric Dumazet, Herbert Xu, Thomas Graf, netdev, linux-kernel,
David S. Miller
In-Reply-To: <20180604181609.sssd4ao6wfwj3il6@netronome.com>
[-- Attachment #1: Type: text/plain, Size: 1228 bytes --]
On Mon, Jun 04 2018, Simon Horman wrote:
> On Mon, Jun 04, 2018 at 12:52:54PM +1000, NeilBrown wrote:
>>
>> Native bit_spin_locks are not tracked by lockdep.
>>
>> The bit_spin_locks used for rhashtable buckets are local
>> to the rhashtable implementation, so there is little opportunity
>> for the sort of misuse that lockdep might detect.
>> However locks are held while a hash function or compare
>> function is called, and if one of these took a lock,
>> a misbehaviour is possible.
>>
>> As it is quite easy to add lockdep support this unlikely
>> possibility see to be enough justification.
>
> nit: s/see/seems/
>
Thanks :-) I've made that change for when I formally submit.
Thanks,
NeilBrown
>>
>> So create a lockdep class for bucket bit_spin_lock as attach
>> through a lockdep_map in each bucket_table.
>>
>> With the 'nested' annotation in rhashtable_rehash_one(), lockdep
>> correctly reports a possible problem as this lock it taken
>> while another bucket lock (in another table) is held. This
>> confirms that the added support works.
>> With the correct nested annotation in place, lockdep reports
>> no problems.
>>
>> Signed-off-by: NeilBrown <neilb@suse.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 0/2] net: phy: improve PM handling of PHY/MDIO
From: Andrew Lunn @ 2018-06-04 21:48 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org
In-Reply-To: <b8f7b42e-791a-5997-d5eb-16f649738421@gmail.com>
On Sat, Jun 02, 2018 at 10:33:36PM +0200, Heiner Kallweit wrote:
> Current implementation of MDIO bus PM ops doesn't actually implement
> bus-specific PM ops but just calls PM ops defined on a device level
> what doesn't seem to be fully in line with the core PM model.
>
> When looking e.g. at __device_suspend() the PM core looks for PM ops
> of a device in a specific order:
> 1. device PM domain
> 2. device type
> 3. device class
> 4. device bus
>
> I think it has good reason that there's no PM ops on device level.
> The situation can be improved by modeling PHY's as device type of
> a MDIO device. If for some other type of MDIO device PM ops are
> needed, it could be modeled as struct device_type as well.
Hi Heiner
I tested that the files in /sys/class/bus/mdio/devices/* are still
there. And also not there for MDIO devices which are not PHYs,
e.g. Ethernet switches.
I don't have any boards which do PM. So i cannot test suspend/resume.
I also took a look at drivers/net/dsa/qca8k.c. This is an MDIO switch
which has PM operations. I don't think this change will break it.
I would prefer a bit more testing, but i guess that is what -rc
kernels are for.
Tested-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [net-next 00/12][pull request] Intel Wired LAN Driver Updates 2018-06-04
From: Alexander Duyck @ 2018-06-04 22:02 UTC (permalink / raw)
To: David Miller, Björn Töpel, Karlsson, Magnus,
Alexei Starovoitov, Daniel Borkmann
Cc: Or Gerlitz, Jeff Kirsher, Netdev
In-Reply-To: <20180604.172717.1793799218358789924.davem@davemloft.net>
On Mon, Jun 4, 2018 at 2:27 PM, David Miller <davem@davemloft.net> wrote:
> From: Or Gerlitz <gerlitz.or@gmail.com>
> Date: Tue, 5 Jun 2018 00:11:35 +0300
>
>> Just to make sure, is the AF_XDP ZC (Zero Copy) UAPI going to be
>> merged for this window -- AFAIU from [1], it's still under
>> examination/development/research for non Intel HWs, am I correct or
>> this is going to get in now?
>
> All of the pending AF_XDP changes will be merged this merge window.
>
> I think Intel folks need to review things as fast as possible because
> I pretty much refuse to revert the series or disable it in Kconfig at
> this point.
>
> Thank you.
My understanding of things is that the current AF_XDP patches were
going to be updated to have more of a model agnostic API such that
they would work for either the "typewriter" mode or the descriptor
ring based approach. The current plan was to have the zero copy
patches be a follow-on after the vendor agnostic API bits in the
descriptors and such had been sorted out. I believe you guys have the
descriptor fixes already right?
In my opinion the i40e code isn't mature enough yet to really go into
anything other than maybe net-next in a couple weeks. We are going to
need a while to get adequate testing in order to flush out all the
bugs and performance regressions we are likely to see coming out of
this change.
- Alex
^ permalink raw reply
* Re: [PATCH net-next 0/2] net: phy: improve PM handling of PHY/MDIO
From: Florian Fainelli @ 2018-06-04 22:06 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit; +Cc: David Miller, netdev@vger.kernel.org
In-Reply-To: <20180604214829.GA14873@lunn.ch>
On 06/04/2018 02:48 PM, Andrew Lunn wrote:
> On Sat, Jun 02, 2018 at 10:33:36PM +0200, Heiner Kallweit wrote:
>> Current implementation of MDIO bus PM ops doesn't actually implement
>> bus-specific PM ops but just calls PM ops defined on a device level
>> what doesn't seem to be fully in line with the core PM model.
>>
>> When looking e.g. at __device_suspend() the PM core looks for PM ops
>> of a device in a specific order:
>> 1. device PM domain
>> 2. device type
>> 3. device class
>> 4. device bus
>>
>> I think it has good reason that there's no PM ops on device level.
>> The situation can be improved by modeling PHY's as device type of
>> a MDIO device. If for some other type of MDIO device PM ops are
>> needed, it could be modeled as struct device_type as well.
>
> Hi Heiner
>
> I tested that the files in /sys/class/bus/mdio/devices/* are still
> there. And also not there for MDIO devices which are not PHYs,
> e.g. Ethernet switches.
>
> I don't have any boards which do PM. So i cannot test suspend/resume.
>
> I also took a look at drivers/net/dsa/qca8k.c. This is an MDIO switch
> which has PM operations. I don't think this change will break it.
I don't think so, but I will give it a spin on a board that has system
wide suspend/resume support. Might take a few hours.
>
> I would prefer a bit more testing, but i guess that is what -rc
> kernels are for.
>
> Tested-by: Andrew Lunn <andrew@lunn.ch>
>
> Andrew
>
--
Florian
^ permalink raw reply
* Re: [PATCH 10/18] rhashtable: remove rhashtable_walk_peek()
From: Tom Herbert @ 2018-06-04 22:13 UTC (permalink / raw)
To: Tom Herbert
Cc: NeilBrown, Herbert Xu, Thomas Graf,
Linux Kernel Network Developers, LKML
In-Reply-To: <CALx6S35sf4oj=x73SfhUxi1e9-u4Stxgr9BAj5E7KKmbufntwg@mail.gmail.com>
On Mon, Jun 4, 2018 at 2:31 PM, Tom Herbert <tom@herbertland.com> wrote:
> On Sun, Jun 3, 2018 at 7:09 PM, NeilBrown <neilb@suse.com> wrote:
>> On Sun, Jun 03 2018, Tom Herbert wrote:
>>
>>> On Sun, Jun 3, 2018 at 5:30 PM, NeilBrown <neilb@suse.com> wrote:
>>>> On Sat, Jun 02 2018, Herbert Xu wrote:
>>>>
>>>>> On Fri, Jun 01, 2018 at 02:44:09PM +1000, NeilBrown wrote:
>>>>>> This function has a somewhat confused behavior that is not properly
>>>>>> described by the documentation.
>>>>>> Sometimes is returns the previous object, sometimes it returns the
>>>>>> next one.
>>>>>> Sometimes it changes the iterator, sometimes it doesn't.
>>>>>>
>>>>>> This function is not currently used and is not worth keeping, so
>>>>>> remove it.
>>>>>>
>>>>>> A future patch will introduce a new function with a
>>>>>> simpler interface which can meet the same need that
>>>>>> this was added for.
>>>>>>
>>>>>> Signed-off-by: NeilBrown <neilb@suse.com>
>>>>>
>>>>> Please keep Tom Herbert in the loop. IIRC he had an issue with
>>>>> this patch.
>>>>
>>>> Yes you are right - sorry for forgetting to add Tom.
>>>>
>>>> My understanding of where this issue stands is that Tom raised issue and
>>>> asked for clarification, I replied, nothing further happened.
>>>>
>>>> It summary, my position is that:
>>>> - most users of my new rhashtable_walk_prev() will use it like
>>>> rhasthable_talk_prev() ?: rhashtable_walk_next()
>>>> which is close to what rhashtable_walk_peek() does
>>>> - I know of no use-case that could not be solved if we only had
>>>> the combined operation
>>>> - BUT it is hard to document the combined operation, as it really
>>>> does two things. If it is hard to document, then it might be
>>>> hard to understand.
>>>>
>>>> So provide the most understandable/maintainable solution, I think
>>>> we should provide rhashtable_walk_prev() as a separate interface.
>>>>
>>> I'm still missing why requiring two API operations instead of one is
>>> simpler or easier to document. Also, I disagree that
>>> rhashtable_walk_peek does two things-- it just does one which is to
>>> return the current element in the walk without advancing to the next
>>> one. The fact that the iterator may or may not move is immaterial in
>>> the API, that is an implementation detail. In fact, it's conceivable
>>> that we might completely reimplement this someday such that the
>>> iterator works completely differently implementation semantics but the
>>> API doesn't change. Also the naming in your proposal is confusing,
>>> we'd have operations to get the previous, and the next next object--
>>> so the user may ask where's the API to get the current object in the
>>> walk? The idea that we get it by first trying to get the previous
>>> object, and then if that fails getting the next object seems
>>> counterintuitive.
>>
>> To respond to your points out of order:
>>
>> - I accept that "rhashtable_walk_prev" is not a perfect name. It
>> suggests a stronger symmetry with rhasthable_walk_next than actually
>> exist. I cannot think of a better name, but I think the
>> description "Return the previously returned object if it is
>> still in the table" is clear and simple and explains the name.
>> I'm certainly open to suggestions for a better name.
>>
>> - I don't think it is meaningful to talk about a "current" element in a
>> table where asynchronous insert/remove is to be expected.
>> The best we can hope for is a "current location" is the sequence of
>> objects in the table - a location which is after some objects and
>> before all others. rhashtable_walk_next() returns the next object
>> after the current location, and advances the location pointer past
>> that object.
>> rhashtable_walk_prev() *doesn't* return the previous object in the
>> table. It returns the previously returned object. ("previous" in
>> time, but not in space, if you like).
>>
>> - rhashtable_walk_peek() currently does one of two different things.
>> It either returns the previously returned object (iter->p) if that
>> is still in the table, or it find the next object, steps over it, and
>> returns it.
>>
>> - I would like to suggest that when an API acts on a iterator object,
>> the question of whether or not the iterator is advanced *must* be a
>> fundamental question, not one that might change from time to time.
>>
>> Maybe a useful way forward would be for you to write documentation for
>> the rhashtable_walk_peek() interface which correctly describes what it
>> does and how it is used. Given that, I can implement that interface
>> with the stability improvements that I'm working on.
>>
>
> Here's how it's documented currently:
>
> "rhashtable_walk_peek - Return the next object but don't advance the iterator"
>
> I don't see what is incorrect about that. Peek returns the next object
> in the walk, however does not move the iterator past that object, so
> sucessive calls to peek return the same object. In other words it's a
> way to inspect the next object but not "consume" it. This is what is
> needed when netlink returns in the middle of a walk. The last object
> retrieved from the table may not have been processed completely, so it
> needs to be the first one processed on the next invocation to netlink.
>
> This is also easily distinguishable from
>
> "rhashtable_walk_next - Return the next object and advance the iterator"
>
> Where the only difference is that peek and walk is that, walk advances
> the iterator and peek does not. Hence why "peek" is a descriptive name
> for what is happening.
>
btw, we are using rhashtable_walk_peek with ILA code that hasn't been
upstreamed yet. I'll (re)post the patches shortly, this demonstates
why we need the peek functionality. If you think that
rhashtable_walk_peek is nothing more than an inline that does "return
rhashtable_walk_prev(iter) ? : rhashtable_walk_next(iter);" then maybe
we could redefine rhashtable_walk_peek to be that. But, then I'll ask
what the use case is for rhashtable_walk_prev as a standalone
function? We created rhashtable_walk_peek for the netlink walk problem
and I don't think any of the related use cases would ever call
rhashtable_walk_prev without the rhashtable_walk_next fallback.
Tom
> Tom
>
>> Thanks,
>> NeilBrown
^ permalink raw reply
* [PATCH net-next 0/3] devlink: Add extack messages for reload and port split/unsplit
From: dsahern @ 2018-06-04 22:15 UTC (permalink / raw)
To: netdev; +Cc: idosch, jiri, jakub.kicinski, David Ahern
From: David Ahern <dsahern@gmail.com>
Patch 1 adds extack arg to reload, port_split and port_unsplit devlink
operations.
Patch 2 adds extack messages for reload operation in netdevsim.
Patch 3 adds extack messages to port split/unsplit in mlxsw driver.
David Ahern (3):
devlink: Add extack to reload and port_{un,}split operations
netdevsim: Add extack error message for devlink reload
mlxsw: Add extack messages for port_{un,}split failures
drivers/net/ethernet/mellanox/mlxsw/core.c | 23 ++++++++++++++++-------
drivers/net/ethernet/mellanox/mlxsw/core.h | 5 +++--
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 13 +++++++++++--
drivers/net/ethernet/netronome/nfp/nfp_devlink.c | 5 +++--
drivers/net/netdevsim/devlink.c | 7 ++++---
drivers/net/netdevsim/fib.c | 9 ++++++---
drivers/net/netdevsim/netdevsim.h | 3 ++-
include/net/devlink.h | 7 ++++---
net/core/devlink.c | 18 ++++++++++--------
9 files changed, 59 insertions(+), 31 deletions(-)
--
2.11.0
^ permalink raw reply
* [PATCH net-next 1/3] devlink: Add extack to reload and port_{un,}split operations
From: dsahern @ 2018-06-04 22:15 UTC (permalink / raw)
To: netdev; +Cc: idosch, jiri, jakub.kicinski, David Ahern
In-Reply-To: <20180604221503.20329-1-dsahern@kernel.org>
From: David Ahern <dsahern@gmail.com>
Add extack argument to reload, port_split and port_unsplit operations.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
drivers/net/ethernet/mellanox/mlxsw/core.c | 9 ++++++---
drivers/net/ethernet/netronome/nfp/nfp_devlink.c | 5 +++--
drivers/net/netdevsim/devlink.c | 3 ++-
include/net/devlink.h | 7 ++++---
net/core/devlink.c | 18 ++++++++++--------
5 files changed, 25 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 8a766fe28fa0..7ed38d80bc08 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -770,7 +770,8 @@ static void mlxsw_core_driver_put(const char *kind)
static int mlxsw_devlink_port_split(struct devlink *devlink,
unsigned int port_index,
- unsigned int count)
+ unsigned int count,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
@@ -782,7 +783,8 @@ static int mlxsw_devlink_port_split(struct devlink *devlink,
}
static int mlxsw_devlink_port_unsplit(struct devlink *devlink,
- unsigned int port_index)
+ unsigned int port_index,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
@@ -963,7 +965,8 @@ mlxsw_devlink_sb_occ_tc_port_bind_get(struct devlink_port *devlink_port,
pool_type, p_cur, p_max);
}
-static int mlxsw_devlink_core_bus_device_reload(struct devlink *devlink)
+static int mlxsw_devlink_core_bus_device_reload(struct devlink *devlink,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
int err;
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index 71c2edd83031..db463e20a876 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
@@ -92,7 +92,7 @@ nfp_devlink_set_lanes(struct nfp_pf *pf, unsigned int idx, unsigned int lanes)
static int
nfp_devlink_port_split(struct devlink *devlink, unsigned int port_index,
- unsigned int count)
+ unsigned int count, struct netlink_ext_ack *extack)
{
struct nfp_pf *pf = devlink_priv(devlink);
struct nfp_eth_table_port eth_port;
@@ -123,7 +123,8 @@ nfp_devlink_port_split(struct devlink *devlink, unsigned int port_index,
}
static int
-nfp_devlink_port_unsplit(struct devlink *devlink, unsigned int port_index)
+nfp_devlink_port_unsplit(struct devlink *devlink, unsigned int port_index,
+ struct netlink_ext_ack *extack)
{
struct nfp_pf *pf = devlink_priv(devlink);
struct nfp_eth_table_port eth_port;
diff --git a/drivers/net/netdevsim/devlink.c b/drivers/net/netdevsim/devlink.c
index bef7db5d129a..e8366cf372ff 100644
--- a/drivers/net/netdevsim/devlink.c
+++ b/drivers/net/netdevsim/devlink.c
@@ -147,7 +147,8 @@ static int devlink_resources_register(struct devlink *devlink)
return err;
}
-static int nsim_devlink_reload(struct devlink *devlink)
+static int nsim_devlink_reload(struct devlink *devlink,
+ struct netlink_ext_ack *extack)
{
enum nsim_resource_id res_ids[] = {
NSIM_RESOURCE_IPV4_FIB, NSIM_RESOURCE_IPV4_FIB_RULES,
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 9686a1aa4ec9..e336ea9c73df 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -296,12 +296,13 @@ struct devlink_resource {
#define DEVLINK_RESOURCE_ID_PARENT_TOP 0
struct devlink_ops {
- int (*reload)(struct devlink *devlink);
+ int (*reload)(struct devlink *devlink, struct netlink_ext_ack *extack);
int (*port_type_set)(struct devlink_port *devlink_port,
enum devlink_port_type port_type);
int (*port_split)(struct devlink *devlink, unsigned int port_index,
- unsigned int count);
- int (*port_unsplit)(struct devlink *devlink, unsigned int port_index);
+ unsigned int count, struct netlink_ext_ack *extack);
+ int (*port_unsplit)(struct devlink *devlink, unsigned int port_index,
+ struct netlink_ext_ack *extack);
int (*sb_pool_get)(struct devlink *devlink, unsigned int sb_index,
u16 pool_index,
struct devlink_sb_pool_info *pool_info);
diff --git a/net/core/devlink.c b/net/core/devlink.c
index f75ee022e6b2..22099705cc41 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -702,12 +702,13 @@ static int devlink_nl_cmd_port_set_doit(struct sk_buff *skb,
return 0;
}
-static int devlink_port_split(struct devlink *devlink,
- u32 port_index, u32 count)
+static int devlink_port_split(struct devlink *devlink, u32 port_index,
+ u32 count, struct netlink_ext_ack *extack)
{
if (devlink->ops && devlink->ops->port_split)
- return devlink->ops->port_split(devlink, port_index, count);
+ return devlink->ops->port_split(devlink, port_index, count,
+ extack);
return -EOPNOTSUPP;
}
@@ -724,14 +725,15 @@ static int devlink_nl_cmd_port_split_doit(struct sk_buff *skb,
port_index = nla_get_u32(info->attrs[DEVLINK_ATTR_PORT_INDEX]);
count = nla_get_u32(info->attrs[DEVLINK_ATTR_PORT_SPLIT_COUNT]);
- return devlink_port_split(devlink, port_index, count);
+ return devlink_port_split(devlink, port_index, count, info->extack);
}
-static int devlink_port_unsplit(struct devlink *devlink, u32 port_index)
+static int devlink_port_unsplit(struct devlink *devlink, u32 port_index,
+ struct netlink_ext_ack *extack)
{
if (devlink->ops && devlink->ops->port_unsplit)
- return devlink->ops->port_unsplit(devlink, port_index);
+ return devlink->ops->port_unsplit(devlink, port_index, extack);
return -EOPNOTSUPP;
}
@@ -745,7 +747,7 @@ static int devlink_nl_cmd_port_unsplit_doit(struct sk_buff *skb,
return -EINVAL;
port_index = nla_get_u32(info->attrs[DEVLINK_ATTR_PORT_INDEX]);
- return devlink_port_unsplit(devlink, port_index);
+ return devlink_port_unsplit(devlink, port_index, info->extack);
}
static int devlink_nl_sb_fill(struct sk_buff *msg, struct devlink *devlink,
@@ -2599,7 +2601,7 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
NL_SET_ERR_MSG_MOD(info->extack, "resources size validation failed");
return err;
}
- return devlink->ops->reload(devlink);
+ return devlink->ops->reload(devlink, info->extack);
}
static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = {
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 2/3] netdevsim: Add extack error message for devlink reload
From: dsahern @ 2018-06-04 22:15 UTC (permalink / raw)
To: netdev; +Cc: idosch, jiri, jakub.kicinski, David Ahern
In-Reply-To: <20180604221503.20329-1-dsahern@kernel.org>
From: David Ahern <dsahern@gmail.com>
devlink reset command can fail if a FIB resource limit is set to a value
lower than the current occupancy. Return a proper message indicating the
reason for the failure.
$ devlink resource sh netdevsim/netdevsim0
netdevsim/netdevsim0:
name IPv4 size unlimited unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none
resources:
name fib size unlimited occ 43 unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none
name fib-rules size unlimited occ 4 unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none
name IPv6 size unlimited unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none
resources:
name fib size unlimited occ 54 unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none
name fib-rules size unlimited occ 3 unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none
$ devlink resource set netdevsim/netdevsim0 path /IPv4/fib size 40
$ devlink dev reload netdevsim/netdevsim0
Error: netdevsim: New size is less than current occupancy.
devlink answers: Invalid argument
Signed-off-by: David Ahern <dsahern@gmail.com>
---
drivers/net/netdevsim/devlink.c | 4 ++--
drivers/net/netdevsim/fib.c | 9 ++++++---
drivers/net/netdevsim/netdevsim.h | 3 ++-
3 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/net/netdevsim/devlink.c b/drivers/net/netdevsim/devlink.c
index e8366cf372ff..ba663e5af168 100644
--- a/drivers/net/netdevsim/devlink.c
+++ b/drivers/net/netdevsim/devlink.c
@@ -163,7 +163,7 @@ static int nsim_devlink_reload(struct devlink *devlink,
err = devlink_resource_size_get(devlink, res_ids[i], &val);
if (!err) {
- err = nsim_fib_set_max(net, res_ids[i], val);
+ err = nsim_fib_set_max(net, res_ids[i], val, extack);
if (err)
return err;
}
@@ -181,7 +181,7 @@ static void nsim_devlink_net_reset(struct net *net)
int i;
for (i = 0; i < ARRAY_SIZE(res_ids); ++i) {
- if (nsim_fib_set_max(net, res_ids[i], (u64)-1)) {
+ if (nsim_fib_set_max(net, res_ids[i], (u64)-1, NULL)) {
pr_err("Failed to reset limit for resource %u\n",
res_ids[i]);
}
diff --git a/drivers/net/netdevsim/fib.c b/drivers/net/netdevsim/fib.c
index 9bfe9e151e13..f61d094746c0 100644
--- a/drivers/net/netdevsim/fib.c
+++ b/drivers/net/netdevsim/fib.c
@@ -64,7 +64,8 @@ u64 nsim_fib_get_val(struct net *net, enum nsim_resource_id res_id, bool max)
return max ? entry->max : entry->num;
}
-int nsim_fib_set_max(struct net *net, enum nsim_resource_id res_id, u64 val)
+int nsim_fib_set_max(struct net *net, enum nsim_resource_id res_id, u64 val,
+ struct netlink_ext_ack *extack)
{
struct nsim_fib_data *fib_data = net_generic(net, nsim_fib_net_id);
struct nsim_fib_entry *entry;
@@ -90,10 +91,12 @@ int nsim_fib_set_max(struct net *net, enum nsim_resource_id res_id, u64 val)
/* not allowing a new max to be less than curren occupancy
* --> no means of evicting entries
*/
- if (val < entry->num)
+ if (val < entry->num) {
+ NL_SET_ERR_MSG_MOD(extack, "New size is less than current occupancy");
err = -EINVAL;
- else
+ } else {
entry->max = val;
+ }
return err;
}
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 3a8581af3b85..8ca50b72c328 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -126,7 +126,8 @@ void nsim_devlink_exit(void);
int nsim_fib_init(void);
void nsim_fib_exit(void);
u64 nsim_fib_get_val(struct net *net, enum nsim_resource_id res_id, bool max);
-int nsim_fib_set_max(struct net *net, enum nsim_resource_id res_id, u64 val);
+int nsim_fib_set_max(struct net *net, enum nsim_resource_id res_id, u64 val,
+ struct netlink_ext_ack *extack);
#else
static inline int nsim_devlink_setup(struct netdevsim *ns)
{
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 3/3] mlxsw: Add extack messages for port_{un,}split failures
From: dsahern @ 2018-06-04 22:15 UTC (permalink / raw)
To: netdev; +Cc: idosch, jiri, jakub.kicinski, David Ahern
In-Reply-To: <20180604221503.20329-1-dsahern@kernel.org>
From: David Ahern <dsahern@gmail.com>
Return messages in extack for port split/unsplit errors. e.g.,
$ devlink port split swp1s1 count 4
Error: mlxsw_spectrum: Port cannot be split further.
devlink answers: Invalid argument
$ devlink port unsplit swp4
Error: mlxsw_spectrum: Port was not split.
devlink answers: Invalid argument
Signed-off-by: David Ahern <dsahern@gmail.com>
---
drivers/net/ethernet/mellanox/mlxsw/core.c | 14 ++++++++++----
drivers/net/ethernet/mellanox/mlxsw/core.h | 5 +++--
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 13 +++++++++++--
3 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 7ed38d80bc08..f9c724752a32 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -775,11 +775,14 @@ static int mlxsw_devlink_port_split(struct devlink *devlink,
{
struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
- if (port_index >= mlxsw_core->max_ports)
+ if (port_index >= mlxsw_core->max_ports) {
+ NL_SET_ERR_MSG_MOD(extack, "Port index exceeds maximum number of ports");
return -EINVAL;
+ }
if (!mlxsw_core->driver->port_split)
return -EOPNOTSUPP;
- return mlxsw_core->driver->port_split(mlxsw_core, port_index, count);
+ return mlxsw_core->driver->port_split(mlxsw_core, port_index, count,
+ extack);
}
static int mlxsw_devlink_port_unsplit(struct devlink *devlink,
@@ -788,11 +791,14 @@ static int mlxsw_devlink_port_unsplit(struct devlink *devlink,
{
struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
- if (port_index >= mlxsw_core->max_ports)
+ if (port_index >= mlxsw_core->max_ports) {
+ NL_SET_ERR_MSG_MOD(extack, "Port index exceeds maximum number of ports");
return -EINVAL;
+ }
if (!mlxsw_core->driver->port_unsplit)
return -EOPNOTSUPP;
- return mlxsw_core->driver->port_unsplit(mlxsw_core, port_index);
+ return mlxsw_core->driver->port_unsplit(mlxsw_core, port_index,
+ extack);
}
static int
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index 4a8d4c7f89d9..552cfa29c2f7 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -274,8 +274,9 @@ struct mlxsw_driver {
int (*port_type_set)(struct mlxsw_core *mlxsw_core, u8 local_port,
enum devlink_port_type new_type);
int (*port_split)(struct mlxsw_core *mlxsw_core, u8 local_port,
- unsigned int count);
- int (*port_unsplit)(struct mlxsw_core *mlxsw_core, u8 local_port);
+ unsigned int count, struct netlink_ext_ack *extack);
+ int (*port_unsplit)(struct mlxsw_core *mlxsw_core, u8 local_port,
+ struct netlink_ext_ack *extack);
int (*sb_pool_get)(struct mlxsw_core *mlxsw_core,
unsigned int sb_index, u16 pool_index,
struct devlink_sb_pool_info *pool_info);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index fc39f22e5c70..1b6d930e452d 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3092,7 +3092,8 @@ static void mlxsw_sp_port_unsplit_create(struct mlxsw_sp *mlxsw_sp,
}
static int mlxsw_sp_port_split(struct mlxsw_core *mlxsw_core, u8 local_port,
- unsigned int count)
+ unsigned int count,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
struct mlxsw_sp_port *mlxsw_sp_port;
@@ -3104,6 +3105,7 @@ static int mlxsw_sp_port_split(struct mlxsw_core *mlxsw_core, u8 local_port,
if (!mlxsw_sp_port) {
dev_err(mlxsw_sp->bus_info->dev, "Port number \"%d\" does not exist\n",
local_port);
+ NL_SET_ERR_MSG_MOD(extack, "Port number does not exist");
return -EINVAL;
}
@@ -3112,11 +3114,13 @@ static int mlxsw_sp_port_split(struct mlxsw_core *mlxsw_core, u8 local_port,
if (count != 2 && count != 4) {
netdev_err(mlxsw_sp_port->dev, "Port can only be split into 2 or 4 ports\n");
+ NL_SET_ERR_MSG_MOD(extack, "Port can only be split into 2 or 4 ports");
return -EINVAL;
}
if (cur_width != MLXSW_PORT_MODULE_MAX_WIDTH) {
netdev_err(mlxsw_sp_port->dev, "Port cannot be split further\n");
+ NL_SET_ERR_MSG_MOD(extack, "Port cannot be split further");
return -EINVAL;
}
@@ -3125,6 +3129,7 @@ static int mlxsw_sp_port_split(struct mlxsw_core *mlxsw_core, u8 local_port,
base_port = local_port;
if (mlxsw_sp->ports[base_port + 1]) {
netdev_err(mlxsw_sp_port->dev, "Invalid split configuration\n");
+ NL_SET_ERR_MSG_MOD(extack, "Invalid split configuration");
return -EINVAL;
}
} else {
@@ -3132,6 +3137,7 @@ static int mlxsw_sp_port_split(struct mlxsw_core *mlxsw_core, u8 local_port,
if (mlxsw_sp->ports[base_port + 1] ||
mlxsw_sp->ports[base_port + 3]) {
netdev_err(mlxsw_sp_port->dev, "Invalid split configuration\n");
+ NL_SET_ERR_MSG_MOD(extack, "Invalid split configuration");
return -EINVAL;
}
}
@@ -3153,7 +3159,8 @@ static int mlxsw_sp_port_split(struct mlxsw_core *mlxsw_core, u8 local_port,
return err;
}
-static int mlxsw_sp_port_unsplit(struct mlxsw_core *mlxsw_core, u8 local_port)
+static int mlxsw_sp_port_unsplit(struct mlxsw_core *mlxsw_core, u8 local_port,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
struct mlxsw_sp_port *mlxsw_sp_port;
@@ -3165,11 +3172,13 @@ static int mlxsw_sp_port_unsplit(struct mlxsw_core *mlxsw_core, u8 local_port)
if (!mlxsw_sp_port) {
dev_err(mlxsw_sp->bus_info->dev, "Port number \"%d\" does not exist\n",
local_port);
+ NL_SET_ERR_MSG_MOD(extack, "Port number does not exist");
return -EINVAL;
}
if (!mlxsw_sp_port->split) {
netdev_err(mlxsw_sp_port->dev, "Port wasn't split\n");
+ NL_SET_ERR_MSG_MOD(extack, "Port was not split");
return -EINVAL;
}
--
2.11.0
^ permalink raw reply related
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