* Re: linux-next: Tree for Feb 26 (net/sched/sch_pie.c)
From: Randy Dunlap @ 2019-02-26 16:21 UTC (permalink / raw)
To: Stephen Rothwell, Linux Next Mailing List
Cc: Linux Kernel Mailing List, netdev@vger.kernel.org,
Vijay Subramanian, Mythili Prabhu
In-Reply-To: <20190226190859.624e21f0@canb.auug.org.au>
On 2/26/19 12:08 AM, Stephen Rothwell wrote:
> Hi all,
>
> Changes since 20190225:
>
on i386:
ld: net/sched/sch_pie.o: in function `pie_timer':
sch_pie.c:(.text+0x604): undefined reference to `__udivdi3'
--
~Randy
^ permalink raw reply
* Re: [PATCH 0/4] mwifiex PCI/wake-up interrupt fixes
From: Ard Biesheuvel @ 2019-02-26 16:21 UTC (permalink / raw)
To: Marc Zyngier
Cc: Amitkumar Karwar, Enric Balletbo i Serra, Ganapathi Bhat,
Heiko Stuebner, Kalle Valo, Nishant Sarmukadam, Rob Herring,
Xinming Hu, Devicetree List, <netdev@vger.kernel.org>,
<linux-wireless@vger.kernel.org>, Linux Kernel Mailing List,
linux-rockchip, David S. Miller, linux-arm-kernel
In-Reply-To: <5310b73b-4821-6dff-b9c0-34c59fb7fd72@arm.com>
On Mon, 25 Feb 2019 at 15:53, Marc Zyngier <marc.zyngier@arm.com> wrote:
>
> Hi Ard,
>
> On 25/02/2019 12:45, Ard Biesheuvel wrote:
> > On Sun, 24 Feb 2019 at 15:08, Marc Zyngier <marc.zyngier@arm.com> wrote:
> >>
> >> For quite some time, I wondered why the PCI mwifiex device built in my
> >> Chromebook was unable to use the good old legacy interrupts. But as MSIs
> >> were working fine, I never really bothered investigating. I finally had a
> >> look, and the result isn't very pretty.
> >>
> >> On this machine (rk3399-based kevin), the wake-up interrupt is described as
> >> such:
> >>
> >> &pci_rootport {
> >> mvl_wifi: wifi@0,0 {
> >> compatible = "pci1b4b,2b42";
> >> reg = <0x83010000 0x0 0x00000000 0x0 0x00100000
> >> 0x83010000 0x0 0x00100000 0x0 0x00100000>;
> >> interrupt-parent = <&gpio0>;
> >> interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
> >> pinctrl-names = "default";
> >> pinctrl-0 = <&wlan_host_wake_l>;
> >> wakeup-source;
> >> };
> >> };
> >>
> >> Note how the interrupt is part of the properties directly attached to the
> >> PCI node. And yet, this interrupt has nothing to do with a PCI legacy
> >> interrupt, as it is attached to the wake-up widget that bypasses the PCIe RC
> >> altogether (Yay for the broken design!). This is in total violation of the
> >> IEEE Std 1275-1994 spec[1], which clearly documents that such interrupt
> >> specifiers describe the PCI device interrupts, and must obey the
> >> INT-{A,B,C,D} mapping. Oops!
> >>
> >
> > The mapping of legacy PCIe INTx interrupts onto wired system
> > interrupts is a property of the PCIe host controller, not of a
> > particular PCIe device. So I would argue that the code is broken here
> > as well: it should never attempt to interpret 'interrupt' properties
> > at the PCI device level as having any bearing on how legacy interrupts
> > are routed.
>
> OpenFirmware says that this node contains the interrupt number of the
> device (4.1.1. Open Firmware-defined Properties for Child Nodes). The
> trick is that this property is generated *from* the device, and not set
> in stone.
>
> DT, on the other hand, takes whatever is described there and uses it as
> the gospel to configure the OS, no matter how the PCI device is actually
> configured. If the two don't match (like in this case), things break.
> This is the "DT describes the HW" mantra, for (sometimes) better or
> (more generally) worse.
>
> What the DT code does is to interpret the whole interrupt specifier,
> *including the interrupt-parent*. And that feels wrong. It should always
> be in the context of the host controller. But on the other side, the DT
> code is not in the business of validating the DT either...
>
> It outlines one thing: If you have to interpret per-device PCI
> properties from DT, you're in for serious trouble. I should get some
> better HW.
>
Yeah, it obviously makes no sense at all for the interrupt parent of a
PCI device to deviate from the host bridge's interrupt parent, and
it's quite unfortunate that we can't simply ban it now that the cat is
out of the bag already.
Arguably, the wake up widget is not part of the PCI device, but I have
no opinion as to whether it is better modeling it as a sub device as
you are proposing or as an entirely separate device referenced via a
phandle.
^ permalink raw reply
* Re: [PATCH net-next] net: remove unused struct inet_frag_queue.fragments field
From: Stefan Schmidt @ 2019-02-26 16:22 UTC (permalink / raw)
To: Peter Oskolkov, David Miller, netdev; +Cc: Peter Oskolkov, linux-wpan
In-Reply-To: <20190226014346.77719-1-posk@google.com>
Hello Peter.
On 26.02.19 02:43, Peter Oskolkov wrote:
> Now that all users of struct inet_frag_queue have been converted
> to use 'rb_fragments', remove the unused 'fragments' field.
>
> Build with `make allyesconfig` succeeded. ip_defrag selftest passed.
>
> Signed-off-by: Peter Oskolkov <posk@google.com>
> ---
> include/net/inet_frag.h | 4 +--
> net/ieee802154/6lowpan/reassembly.c | 1 -
> net/ipv4/inet_fragment.c | 44 ++++++++-----------------
> net/ipv4/ip_fragment.c | 2 --
> net/ipv6/netfilter/nf_conntrack_reasm.c | 1 -
> net/ipv6/reassembly.c | 1 -
> 6 files changed, 14 insertions(+), 39 deletions(-)
>
> diff --git a/include/net/inet_frag.h b/include/net/inet_frag.h
> index b02bf737d019..378904ee9129 100644
> --- a/include/net/inet_frag.h
> +++ b/include/net/inet_frag.h
> @@ -56,7 +56,6 @@ struct frag_v6_compare_key {
> * @timer: queue expiration timer
> * @lock: spinlock protecting this frag
> * @refcnt: reference count of the queue
> - * @fragments: received fragments head
> * @rb_fragments: received fragments rb-tree root
> * @fragments_tail: received fragments tail
> * @last_run_head: the head of the last "run". see ip_fragment.c
> @@ -77,8 +76,7 @@ struct inet_frag_queue {
> struct timer_list timer;
> spinlock_t lock;
> refcount_t refcnt;
> - struct sk_buff *fragments; /* used in 6lopwpan IPv6. */
> - struct rb_root rb_fragments; /* Used in IPv4/IPv6. */
> + struct rb_root rb_fragments;
> struct sk_buff *fragments_tail;
> struct sk_buff *last_run_head;
> ktime_t stamp;
> diff --git a/net/ieee802154/6lowpan/reassembly.c b/net/ieee802154/6lowpan/reassembly.c
> index bd61633d2c32..4196bcd4105a 100644
> --- a/net/ieee802154/6lowpan/reassembly.c
> +++ b/net/ieee802154/6lowpan/reassembly.c
> @@ -179,7 +179,6 @@ static int lowpan_frag_reasm(struct lowpan_frag_queue *fq, struct sk_buff *skb,
>
> skb->dev = ldev;
> skb->tstamp = fq->q.stamp;
> - fq->q.fragments = NULL;
> fq->q.rb_fragments = RB_ROOT;
> fq->q.fragments_tail = NULL;
> fq->q.last_run_head = NULL;
> diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
> index 9f69411251d0..737808e27f8b 100644
> --- a/net/ipv4/inet_fragment.c
> +++ b/net/ipv4/inet_fragment.c
> @@ -203,7 +203,6 @@ EXPORT_SYMBOL(inet_frag_rbtree_purge);
>
> void inet_frag_destroy(struct inet_frag_queue *q)
> {
> - struct sk_buff *fp;
> struct netns_frags *nf;
> unsigned int sum, sum_truesize = 0;
> struct inet_frags *f;
> @@ -212,20 +211,9 @@ void inet_frag_destroy(struct inet_frag_queue *q)
> WARN_ON(del_timer(&q->timer) != 0);
>
> /* Release all fragment data. */
> - fp = q->fragments;
> nf = q->net;
> f = nf->f;
> - if (fp) {
> - do {
> - struct sk_buff *xp = fp->next;
> -
> - sum_truesize += fp->truesize;
> - kfree_skb(fp);
> - fp = xp;
> - } while (fp);
> - } else {
> - sum_truesize = inet_frag_rbtree_purge(&q->rb_fragments);
> - }
> + sum_truesize = inet_frag_rbtree_purge(&q->rb_fragments);
> sum = sum_truesize + f->qsize;
>
> call_rcu(&q->rcu, inet_frag_destroy_rcu);
> @@ -489,26 +477,20 @@ EXPORT_SYMBOL(inet_frag_reasm_finish);
>
> struct sk_buff *inet_frag_pull_head(struct inet_frag_queue *q)
> {
> - struct sk_buff *head;
> + struct sk_buff *head, *skb;
>
> - if (q->fragments) {
> - head = q->fragments;
> - q->fragments = head->next;
> - } else {
> - struct sk_buff *skb;
> + head = skb_rb_first(&q->rb_fragments);
> + if (!head)
> + return NULL;
> + skb = FRAG_CB(head)->next_frag;
> + if (skb)
> + rb_replace_node(&head->rbnode, &skb->rbnode,
> + &q->rb_fragments);
> + else
> + rb_erase(&head->rbnode, &q->rb_fragments);
> + memset(&head->rbnode, 0, sizeof(head->rbnode));
> + barrier();
>
> - head = skb_rb_first(&q->rb_fragments);
> - if (!head)
> - return NULL;
> - skb = FRAG_CB(head)->next_frag;
> - if (skb)
> - rb_replace_node(&head->rbnode, &skb->rbnode,
> - &q->rb_fragments);
> - else
> - rb_erase(&head->rbnode, &q->rb_fragments);
> - memset(&head->rbnode, 0, sizeof(head->rbnode));
> - barrier();
> - }
> if (head == q->fragments_tail)
> q->fragments_tail = NULL;
>
> diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
> index 486ecb0aeb87..cf2b0a6a3337 100644
> --- a/net/ipv4/ip_fragment.c
> +++ b/net/ipv4/ip_fragment.c
> @@ -261,7 +261,6 @@ static int ip_frag_reinit(struct ipq *qp)
> qp->q.flags = 0;
> qp->q.len = 0;
> qp->q.meat = 0;
> - qp->q.fragments = NULL;
> qp->q.rb_fragments = RB_ROOT;
> qp->q.fragments_tail = NULL;
> qp->q.last_run_head = NULL;
> @@ -451,7 +450,6 @@ static int ip_frag_reasm(struct ipq *qp, struct sk_buff *skb,
> ip_send_check(iph);
>
> __IP_INC_STATS(net, IPSTATS_MIB_REASMOKS);
> - qp->q.fragments = NULL;
> qp->q.rb_fragments = RB_ROOT;
> qp->q.fragments_tail = NULL;
> qp->q.last_run_head = NULL;
> diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
> index cb1b4772dac0..3de0e9b0a482 100644
> --- a/net/ipv6/netfilter/nf_conntrack_reasm.c
> +++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
> @@ -365,7 +365,6 @@ static int nf_ct_frag6_reasm(struct frag_queue *fq, struct sk_buff *skb,
> skb_network_header_len(skb),
> skb->csum);
>
> - fq->q.fragments = NULL;
> fq->q.rb_fragments = RB_ROOT;
> fq->q.fragments_tail = NULL;
> fq->q.last_run_head = NULL;
> diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
> index 24264d0a4b85..1a832f5e190b 100644
> --- a/net/ipv6/reassembly.c
> +++ b/net/ipv6/reassembly.c
> @@ -304,7 +304,6 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *skb,
> rcu_read_lock();
> __IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMOKS);
> rcu_read_unlock();
> - fq->q.fragments = NULL;
> fq->q.rb_fragments = RB_ROOT;
> fq->q.fragments_tail = NULL;
> fq->q.last_run_head = NULL;
>
For the ieee802154 part:
Acked-by: Stefan Schmidt <stefan@datenfreihafen.org>
regards
Stefan Schmidt
^ permalink raw reply
* Re: [PATCH] bpf: decrease usercnt if bpf_map_new_fd() fails in bpf_map_get_fd_by_id()
From: Daniel Borkmann @ 2019-02-26 16:24 UTC (permalink / raw)
To: zerons, ast; +Cc: netdev, linux-kernel
In-Reply-To: <288b6db2-b14a-4f85-b89c-d873f0f6fcd8@gmail.com>
On 02/26/2019 03:58 PM, zerons wrote:
> On 2/26/19 22:44, Daniel Borkmann wrote:
>> On 02/26/2019 03:15 PM, zerons wrote:
>>> [ Upstream commit c91951f15978f1a0c6b65f063d30f7ea7bc6fb42 ]
>>
>> Thanks for the fix! What do you mean by "upstream commit" above in this context?
>
> This patch is based on that commit, I thought I should mention this.
> Sorry for the confusion.
Ah, no. Usually that line is placed into stable commits that are
cherry-picked to point to the original commit sha in the mainline
tree. In any case, I didn't find c91951f15978, hence my question.
Anyway, for normal patch submissions in future, please don't add
it into the log, we'll remove it this time.
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH net-next] net: remove unused struct inet_frag_queue.fragments field
From: David Miller @ 2019-02-26 16:27 UTC (permalink / raw)
To: posk; +Cc: netdev, posk, linux-wpan
In-Reply-To: <20190226014346.77719-1-posk@google.com>
From: Peter Oskolkov <posk@google.com>
Date: Mon, 25 Feb 2019 17:43:46 -0800
> Now that all users of struct inet_frag_queue have been converted
> to use 'rb_fragments', remove the unused 'fragments' field.
>
> Build with `make allyesconfig` succeeded. ip_defrag selftest passed.
>
> Signed-off-by: Peter Oskolkov <posk@google.com>
Applied, thanks Peter.
^ permalink raw reply
* Re: AF_XDP design flaws
From: John Fastabend @ 2019-02-26 16:41 UTC (permalink / raw)
To: Maxim Mikityanskiy, netdev@vger.kernel.org, Björn Töpel,
Magnus Karlsson, David S. Miller
Cc: Tariq Toukan, Saeed Mahameed, Eran Ben Elisha
In-Reply-To: <AM6PR05MB5879DF6B2BD7DC426869875ED17B0@AM6PR05MB5879.eurprd05.prod.outlook.com>
On 2/26/19 6:49 AM, Maxim Mikityanskiy wrote:
> Hi everyone,
>
> I would like to discuss some design flaws of AF_XDP socket (XSK) implementation
> in kernel. At the moment I don't see a way to work around them without changing
> the API, so I would like to make sure that I'm not missing anything and to
> suggest and discuss some possible improvements that can be made.
>
> The issues I describe below are caused by the fact that the driver depends on
> the application doing some things, and if the application is
> slow/buggy/malicious, the driver is forced to busy poll because of the lack of a
> notification mechanism from the application side. I will refer to the i40e
> driver implementation a lot, as it is the first implementation of AF_XDP, but
> the issues are general and affect any driver. I already considered trying to fix
> it on driver level, but it doesn't seem possible, so it looks like the behavior
> and implementation of AF_XDP in the kernel has to be changed.
>
> RX side busy polling
> ====================
>
> On the RX side, the driver expects the application to put some descriptors in
> the Fill Ring. There is no way for the application to notify the driver that
> there are more Fill Ring descriptors to take, so the driver is forced to busy
> poll the Fill Ring if it gets empty. E.g., the i40e driver does it in NAPI poll:
>
> int i40e_clean_rx_irq_zc(struct i40e_ring *rx_ring, int budget)
> {
> ...
> failure = failure ||
> !i40e_alloc_rx_buffers_fast_zc(rx_ring,
> cleaned_count);
> ...
> return failure ? budget : (int)total_rx_packets;
> }
>
> Basically, it means that if there are no descriptors in the Fill Ring, NAPI will
> never stop, draining CPU.
>
> Possible cases when it happens
> ------------------------------
>
> 1. The application is slow, it received some frames in the RX Ring, and it is
> still handling the data, so it has no free frames to put to the Fill Ring.
>
> 2. The application is malicious, it opens an XSK and puts no frames to the Fill
> Ring. It can be used as a local DoS attack.
>
> 3. The application is buggy and stops filling the Fill Ring for whatever reason
> (deadlock, waiting for another blocking operation, other bugs).
>
> Although loading an XDP program requires root access, the DoS attack can be
> targeted to setups that already use XDP, i.e. an XDP program is already loaded.
> Even under root, userspace applications should not be able to disrupt system
> stability by just calling normal APIs without an intention to destroy the
> system, and here it happens in case 1.
I believe this is by design. If packets per second is your top priority
(at the expense of power, cpu, etc.) this is the behavior you might
want. To resolve your points if you don't trust the application it
should be isolated to a queue pair and cores so the impact is known and
managed.
That said having a flag to back-off seems like a good idea. But should
be doable as a feature without breaking current API.
>
> Possible way to solve the issue
> -------------------------------
>
> When the driver can't take new Fill Ring frames, it shouldn't busy poll.
> Instead, it signals the failure to the application (e.g., with POLLERR), and
> after that it's up to the application to restart polling (e.g., by calling
> sendto()) after refilling the Fill Ring. The issue with this approach is that it
> changes the API, so we either have to deal with it or to introduce some API
> version field.
See above. I like the idea here though.
>
> TX side getting stuck
> =====================
>
> On the TX side, there is the Completion Ring that the application has to clean.
> If it doesn't, the i40e driver stops taking descriptors from the TX Ring. If the
> application finally completes something, the driver can go on transmitting.
> However, it would require busy polling the Completion Ring (just like with the
> Fill Ring on the RX side). i40e doesn't do it, instead, it relies on the
> application to kick the TX by calling sendto(). The issue is that poll() doesn't
> return POLLOUT in this case, because the TX Ring is full, so the application
> will never call sendto(), and the ring is stuck forever (or at least until
> something triggers NAPI).
>
> Possible way to solve the issue
> -------------------------------
>
> When the driver can't reserve a descriptor in the Completion Ring, it should
> signal the failure to the application (e.g., with POLLERR). The application
> shouldn't call sendto() every time it sees that the number of not completed
> frames is greater than zero (like xdpsock sample does). Instead, the application
> should kick the TX only when it wants to flush the ring, and, in addition, after
> resolving the cause for POLLERR, i.e. after handling Completion Ring entries.
> The API will also have to change with this approach.
>
+1 seems to me this can be done without breaking existing API though.
> Triggering NAPI on a different CPU core
> =======================================
>
> .ndo_xsk_async_xmit runs on a random CPU core, so, to preserve CPU affinity,
> i40e triggers an interrupt to schedule NAPI, instead of calling napi_schedule
> directly. Scheduling NAPI on the correct CPU is what would every driver do, I
> guess, but currently it has to be implemented differently in every driver, and
> it relies on hardware features (the ability to trigger an IRQ).
Ideally the application would be pinned to a core and the traffic
steered to that core using ethtool/tc. Yes it requires a bit of mgmt on
the platform but I think this is needed for best pps numbers.
>
> I suggest introducing a kernel API that would allow triggering NAPI on a given
> CPU. A brief look shows that something like smp_call_function_single_async can
> be used. Advantages:
Assuming you want to avoid pinning cores/traffic for some reason. Could
this be done with some combination of cpumap + af_xdp? I haven't thought
too much about it though. Maybe Bjorn has some ideas.
>
> 1. It lifts the hardware requirement to be able to raise an interrupt on demand.
>
> 2. It would allow to move common code to the kernel (.ndo_xsk_async_xmit).
>
> 3. It is also useful in the situation where CPU affinity changes while being in
> NAPI poll. Currently, i40e and mlx5e try to stop NAPI polling by returning
> a value less than budget if CPU affinity changes. However, there are cases
> (e.g., NAPIF_STATE_MISSED) when NAPI will be rescheduled on a wrong CPU. It's a
> race between the interrupt, which will move NAPI to the correct CPU, and
> __napi_schedule from a wrong CPU. Having an API to schedule NAPI on a given CPU
> will benefit both mlx5e and i40e, because when this situation happens, it kills
> the performance.
How would we know what core to trigger NAPI on?
>
> I would be happy to hear your thoughts about these issues.
At least the first two observations seem incrementally solvable to me
and would be nice improvements. I assume the last case can be resolved
by pinning cores/traffic but for the general case perhaps it is useful.
>
> Thanks,
> Max
>
^ permalink raw reply
* Re: [PATCH net-next v4 0/6] devlink: make ethtool compat reliable
From: David Miller @ 2019-02-26 16:50 UTC (permalink / raw)
To: jakub.kicinski; +Cc: jiri, mkubecek, andrew, f.fainelli, netdev, oss-drivers
In-Reply-To: <20190226033407.32625-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Mon, 25 Feb 2019 19:34:01 -0800
> This is a follow up to the series which added device flash
> updates via devlink. I went with the approach of adding a
> new NDO in the end. It seems to end up looking cleaner.
>
> First patch removes the option to build devlink as a module.
> Users can still decide to not build it, but the module option
> ends up not being worth the maintenance cost.
>
> Next two patches add a NDO which can be used to ask the driver
> to return a devlink instance associated with a given netdev,
> instead of iterating over devlink ports. Drivers which implement
> this NDO must take into account the potential impact on the
> visibility of the devlink instance.
>
> With the new NDO in place we can remove NFP ethtool flash update
> code.
>
> Fifth patch makes sure we hold a reference to dev while
> callbacks are active.
>
> Last but not least the NULL-check of devlink->ops is moved
> to instance allocation time.
>
> Last but not least missing checks for devlink->ops are added.
> There is currently no driver registering devlink without ops,
> so can just fix this in -next.
>
> v2 (Michal): add netdev_to_devlink() in patch 3.
> v3 (Florian):
> - add missing checks for devlink->ops;
> - move locking/holding into devlink_compat_ functions.
> v4 (Jiri):
> - hold devlink_mutex around callbacks (patch 2);
> - require non-NULL ops (patch 6).
Series applied, thanks Jakub.
^ permalink raw reply
* [PATCH net 1/3] ipv4: Return error for RTA_VIA attribute
From: David Ahern @ 2019-02-26 17:00 UTC (permalink / raw)
To: davem; +Cc: netdev, David Ahern
In-Reply-To: <20190226170004.4535-1-dsahern@kernel.org>
From: David Ahern <dsahern@gmail.com>
IPv4 currently does not support nexthops outside of the AF_INET family.
Specifically, it does not handle RTA_VIA attribute. If it is passed
in a route add request, the actual route added only uses the device
which is clearly not what the user intended:
$ ip ro add 172.16.1.0/24 via inet6 2001:db8:1::1 dev eth0
$ ip ro ls
...
172.16.1.0/24 dev eth0
Catch this and fail the route add:
$ ip ro add 172.16.1.0/24 via inet6 2001:db8:1::1 dev eth0
Error: IPv4 does not support RTA_VIA attribute.
Fixes: 03c0566542f4c ("mpls: Netlink commands to add, remove, and dump routes")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/ipv4/fib_frontend.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index fe4f6a624238..ed14ec245584 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -710,6 +710,10 @@ static int rtm_to_fib_config(struct net *net, struct sk_buff *skb,
case RTA_GATEWAY:
cfg->fc_gw = nla_get_be32(attr);
break;
+ case RTA_VIA:
+ NL_SET_ERR_MSG(extack, "IPv4 does not support RTA_VIA attribute");
+ err = -EINVAL;
+ goto errout;
case RTA_PRIORITY:
cfg->fc_priority = nla_get_u32(attr);
break;
--
2.11.0
^ permalink raw reply related
* [PATCH net 2/3] ipv6: Return error for RTA_VIA attribute
From: David Ahern @ 2019-02-26 17:00 UTC (permalink / raw)
To: davem; +Cc: netdev, David Ahern
In-Reply-To: <20190226170004.4535-1-dsahern@kernel.org>
From: David Ahern <dsahern@gmail.com>
IPv6 currently does not support nexthops outside of the AF_INET6 family.
Specifically, it does not handle RTA_VIA attribute. If it is passed
in a route add request, the actual route added only uses the device
which is clearly not what the user intended:
$ ip -6 ro add 2001:db8:2::/64 via inet 172.16.1.1 dev eth0
$ ip ro ls
...
2001:db8:2::/64 dev eth0 metric 1024 pref medium
Catch this and fail the route add:
$ ip -6 ro add 2001:db8:2::/64 via inet 172.16.1.1 dev eth0
Error: IPv6 does not support RTA_VIA attribute.
Fixes: 03c0566542f4c ("mpls: Netlink commands to add, remove, and dump routes")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/ipv6/route.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index ce15dc4ccbfa..b7a620023a52 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -4182,6 +4182,10 @@ static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr *nlh,
cfg->fc_gateway = nla_get_in6_addr(tb[RTA_GATEWAY]);
cfg->fc_flags |= RTF_GATEWAY;
}
+ if (tb[RTA_VIA]) {
+ NL_SET_ERR_MSG(extack, "IPv6 does not support RTA_VIA attribute");
+ goto errout;
+ }
if (tb[RTA_DST]) {
int plen = (rtm->rtm_dst_len + 7) >> 3;
--
2.11.0
^ permalink raw reply related
* [PATCH net 3/3] mpls: Return error for RTA_GATEWAY attribute
From: David Ahern @ 2019-02-26 17:00 UTC (permalink / raw)
To: davem; +Cc: netdev, David Ahern
In-Reply-To: <20190226170004.4535-1-dsahern@kernel.org>
From: David Ahern <dsahern@gmail.com>
MPLS does not support nexthops with an MPLS address family.
Specifically, it does not handle RTA_GATEWAY attribute. Make it
clear by returning an error.
Fixes: 03c0566542f4c ("mpls: Netlink commands to add, remove, and dump routes")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/mpls/af_mpls.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index 7d55d4c04088..fa763e2e50ec 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -1838,6 +1838,9 @@ static int rtm_to_route_config(struct sk_buff *skb,
goto errout;
break;
}
+ case RTA_GATEWAY:
+ NL_SET_ERR_MSG(extack, "MPLS does not support RTA_GATEWAY attribute");
+ goto errout;
case RTA_VIA:
{
if (nla_get_via(nla, &cfg->rc_via_alen,
--
2.11.0
^ permalink raw reply related
* [PATCH net 0/3] net: Fail route add with unsupported nexthop attribute
From: David Ahern @ 2019-02-26 17:00 UTC (permalink / raw)
To: davem; +Cc: netdev, David Ahern
From: David Ahern <dsahern@gmail.com>
RTA_VIA was added for MPLS as a way of specifying a gateway from a
different address family. IPv4 and IPv6 do not currently support RTA_VIA
so using it leads to routes that are not what the user intended. Catch
and fail - returning a proper error message.
MPLS on the other hand does not support RTA_GATEWAY since it does not
make sense to have a nexthop from the MPLS address family. Similarly,
catch and fail - returning a proper error message.
David Ahern (3):
ipv4: Return error for RTA_VIA attribute
ipv6: Return error for RTA_VIA attribute
mpls: Return error for RTA_GATEWAY attribute
net/ipv4/fib_frontend.c | 4 ++++
net/ipv6/route.c | 4 ++++
net/mpls/af_mpls.c | 3 +++
3 files changed, 11 insertions(+)
--
2.11.0
^ permalink raw reply
* Re: [PATCH net-next 0/2] vxlan: create and changelink extack support
From: David Miller @ 2019-02-26 17:01 UTC (permalink / raw)
To: roopa; +Cc: netdev, dsa, sd, johannes
In-Reply-To: <1551160982-32685-1-git-send-email-roopa@cumulusnetworks.com>
From: Roopa Prabhu <roopa@cumulusnetworks.com>
Date: Mon, 25 Feb 2019 22:03:00 -0800
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> This series adds extack support to changelink paths.
> In the process re-factors flag sets to a separate helper.
> Also adds some changelink testcases to rtnetlink.sh
>
> (This series was initially part of another series that
> tried to support changelink for more attributes.
> But after some feedback from sabrina, i have dropped the
> 'support changelink for more attributes' part because some
> of them cannot be supported today or may require additional
> use-case handling code. These can be done separately
> as and when we see the need for it.)
Series applied, thanks Roopa.
^ permalink raw reply
* Re: [PATCH net-next] mlxsw: spectrum: remove set but not used variable 'autoneg_status'
From: David Miller @ 2019-02-26 17:07 UTC (permalink / raw)
To: yuehaibing; +Cc: jiri, idosch, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20190225020328.42510-1-yuehaibing@huawei.com>
From: YueHaibing <yuehaibing@huawei.com>
Date: Mon, 25 Feb 2019 02:03:28 +0000
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/ethernet/mellanox/mlxsw/spectrum.c: In function 'mlxsw_sp_port_get_link_ksettings':
> drivers/net/ethernet/mellanox/mlxsw/spectrum.c:3062:5: warning:
> variable 'autoneg_status' set but not used [-Wunused-but-set-variable]
>
> It's not used since commit 475b33cb66c9 ("mlxsw: spectrum: Remove unsupported
> eth_proto_lp_advertise field in PTYS")
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied.
^ permalink raw reply
* Re: [PATCH 1/3] net: ethernet: add support for PCS and 2.5G speed
From: David Miller @ 2019-02-26 17:09 UTC (permalink / raw)
To: Nicolas.Ferre
Cc: f.fainelli, pthombar, netdev, andrew, hkallweit1, linux-kernel,
rafalc, piotrs, jank, Claudiu.Beznea
In-Reply-To: <a0f7be10-ff9b-c11c-fde9-30876940d719@microchip.com>
From: <Nicolas.Ferre@microchip.com>
Date: Tue, 26 Feb 2019 09:23:04 +0000
> This patch series seem pretty intrusive, so I would like that you wait
> for my explicit ACK before applying even next versions of it.
Ok.
^ permalink raw reply
* Re: [PATCH] net: sched: pie: fix mistake in reference link
From: David Miller @ 2019-02-26 17:12 UTC (permalink / raw)
To: lesliemonis; +Cc: netdev
In-Reply-To: <20190226102331.532-1-lesliemonis@gmail.com>
From: Leslie Monis <lesliemonis@gmail.com>
Date: Tue, 26 Feb 2019 15:53:31 +0530
> Fix the incorrect reference link to RFC 8033
>
> Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH RFC] net: Validate size of non-TSO packets in validate_xmit_skb().
From: David Miller @ 2019-02-26 17:13 UTC (permalink / raw)
To: michael.chan; +Cc: maheshb, edumazet, dja, netdev
In-Reply-To: <1551178601-16564-1-git-send-email-michael.chan@broadcom.com>
From: Michael Chan <michael.chan@broadcom.com>
Date: Tue, 26 Feb 2019 05:56:41 -0500
> There have been reports of oversize UDP packets being sent to the
> driver to be transmitted, causing error conditions. The issue is
> likely caused by the dst of the SKB switching between 'lo' with
> 64K MTU and the hardware device with a smaller MTU. Patches are
> being proposed by Mahesh Bandewar <maheshb@google.com> to fix the
> issue.
>
> Separately, we should add a length check in validate_xmit_skb()
> to drop these oversize packets before they reach the driver.
> This patch only validates non-TSO packets. Complete validation
> of segmented TSO packet size will probably be too slow.
>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
Anything which changes the dst of an SKB really is responsible for
fixing up whatever became "incompatible" in the new path.
So like Eric I want to see this out of the fast path.
^ permalink raw reply
* Re: [PATCH v1 iproute2-next 1/4] rdma: add helper rd_sendrecv_msg()
From: Steve Wise @ 2019-02-26 17:13 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: dsahern, stephen, netdev, linux-rdma
In-Reply-To: <20190223092615.GM23561@mtr-leonro.mtl.com>
On 2/23/2019 3:26 AM, Leon Romanovsky wrote:
> On Thu, Feb 21, 2019 at 08:19:03AM -0800, Steve Wise wrote:
>> This function sends the constructed netlink message and then
>> receives the response, displaying any error text.
>>
>> Change 'rdma dev set' to use it.
>>
>> Signed-off-by: Steve Wise <swise@opengridcomputing.com>
>> ---
>> rdma/dev.c | 2 +-
>> rdma/rdma.h | 1 +
>> rdma/utils.c | 21 +++++++++++++++++++++
>> 3 files changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/rdma/dev.c b/rdma/dev.c
>> index 60ff4b31e320..d2949c378f08 100644
>> --- a/rdma/dev.c
>> +++ b/rdma/dev.c
>> @@ -273,7 +273,7 @@ static int dev_set_name(struct rd *rd)
>> mnl_attr_put_u32(rd->nlh, RDMA_NLDEV_ATTR_DEV_INDEX, rd->dev_idx);
>> mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_DEV_NAME, rd_argv(rd));
>>
>> - return rd_send_msg(rd);
>> + return rd_sendrecv_msg(rd, seq);
>> }
>>
>> static int dev_one_set(struct rd *rd)
>> diff --git a/rdma/rdma.h b/rdma/rdma.h
>> index 547bb5749a39..20be2f12c4f8 100644
>> --- a/rdma/rdma.h
>> +++ b/rdma/rdma.h
>> @@ -115,6 +115,7 @@ bool rd_check_is_key_exist(struct rd *rd, const char *key);
>> */
>> int rd_send_msg(struct rd *rd);
>> int rd_recv_msg(struct rd *rd, mnl_cb_t callback, void *data, uint32_t seq);
>> +int rd_sendrecv_msg(struct rd *rd, unsigned int seq);
>> void rd_prepare_msg(struct rd *rd, uint32_t cmd, uint32_t *seq, uint16_t flags);
>> int rd_dev_init_cb(const struct nlmsghdr *nlh, void *data);
>> int rd_attr_cb(const struct nlattr *attr, void *data);
>> diff --git a/rdma/utils.c b/rdma/utils.c
>> index 069d44fece10..a6f2826c9605 100644
>> --- a/rdma/utils.c
>> +++ b/rdma/utils.c
>> @@ -664,6 +664,27 @@ int rd_recv_msg(struct rd *rd, mnl_cb_t callback, void *data, unsigned int seq)
>> return ret;
>> }
>>
>> +static int null_cb(const struct nlmsghdr *nlh, void *data)
>> +{
>> + return MNL_CB_OK;
>> +}
>> +
>> +int rd_sendrecv_msg(struct rd *rd, unsigned int seq)
>> +{
>> + int ret;
>> +
>> + ret = rd_send_msg(rd);
>> + if (ret) {
>> + perror(NULL);
> This is more or less already done in rd_send_msg() and that function
> prints something in case of execution error. So the missing piece
> is to update rd_recv_msg(), so all places will "magically" print errors
> and not only dev_set_name().
Yea ok.
^ permalink raw reply
* Re: [PATCH 0/4] mwifiex PCI/wake-up interrupt fixes
From: Marc Zyngier @ 2019-02-26 17:14 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Amitkumar Karwar, Enric Balletbo i Serra, Ganapathi Bhat,
Heiko Stuebner, Kalle Valo, Nishant Sarmukadam, Rob Herring,
Xinming Hu, Devicetree List, <netdev@vger.kernel.org>,
<linux-wireless@vger.kernel.org>, Linux Kernel Mailing List,
linux-rockchip, David S. Miller, linux-arm-kernel
In-Reply-To: <CAKv+Gu_=bQyVZqgUnJm9moCcaCcpDrag6NWGEpaR65=pMiTFJQ@mail.gmail.com>
On 26/02/2019 16:21, Ard Biesheuvel wrote:
> On Mon, 25 Feb 2019 at 15:53, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>
>> Hi Ard,
>>
>> On 25/02/2019 12:45, Ard Biesheuvel wrote:
>>> On Sun, 24 Feb 2019 at 15:08, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>>>
>>>> For quite some time, I wondered why the PCI mwifiex device built in my
>>>> Chromebook was unable to use the good old legacy interrupts. But as MSIs
>>>> were working fine, I never really bothered investigating. I finally had a
>>>> look, and the result isn't very pretty.
>>>>
>>>> On this machine (rk3399-based kevin), the wake-up interrupt is described as
>>>> such:
>>>>
>>>> &pci_rootport {
>>>> mvl_wifi: wifi@0,0 {
>>>> compatible = "pci1b4b,2b42";
>>>> reg = <0x83010000 0x0 0x00000000 0x0 0x00100000
>>>> 0x83010000 0x0 0x00100000 0x0 0x00100000>;
>>>> interrupt-parent = <&gpio0>;
>>>> interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
>>>> pinctrl-names = "default";
>>>> pinctrl-0 = <&wlan_host_wake_l>;
>>>> wakeup-source;
>>>> };
>>>> };
>>>>
>>>> Note how the interrupt is part of the properties directly attached to the
>>>> PCI node. And yet, this interrupt has nothing to do with a PCI legacy
>>>> interrupt, as it is attached to the wake-up widget that bypasses the PCIe RC
>>>> altogether (Yay for the broken design!). This is in total violation of the
>>>> IEEE Std 1275-1994 spec[1], which clearly documents that such interrupt
>>>> specifiers describe the PCI device interrupts, and must obey the
>>>> INT-{A,B,C,D} mapping. Oops!
>>>>
>>>
>>> The mapping of legacy PCIe INTx interrupts onto wired system
>>> interrupts is a property of the PCIe host controller, not of a
>>> particular PCIe device. So I would argue that the code is broken here
>>> as well: it should never attempt to interpret 'interrupt' properties
>>> at the PCI device level as having any bearing on how legacy interrupts
>>> are routed.
>>
>> OpenFirmware says that this node contains the interrupt number of the
>> device (4.1.1. Open Firmware-defined Properties for Child Nodes). The
>> trick is that this property is generated *from* the device, and not set
>> in stone.
>>
>> DT, on the other hand, takes whatever is described there and uses it as
>> the gospel to configure the OS, no matter how the PCI device is actually
>> configured. If the two don't match (like in this case), things break.
>> This is the "DT describes the HW" mantra, for (sometimes) better or
>> (more generally) worse.
>>
>> What the DT code does is to interpret the whole interrupt specifier,
>> *including the interrupt-parent*. And that feels wrong. It should always
>> be in the context of the host controller. But on the other side, the DT
>> code is not in the business of validating the DT either...
>>
>> It outlines one thing: If you have to interpret per-device PCI
>> properties from DT, you're in for serious trouble. I should get some
>> better HW.
>>
>
> Yeah, it obviously makes no sense at all for the interrupt parent of a
> PCI device to deviate from the host bridge's interrupt parent, and
> it's quite unfortunate that we can't simply ban it now that the cat is
> out of the bag already.
>
> Arguably, the wake up widget is not part of the PCI device, but I have
> no opinion as to whether it is better modeling it as a sub device as
> you are proposing or as an entirely separate device referenced via a
> phandle.
It is not that clear. The widget seems to be an integral part of the
device, as it is the same basic IP that is used for SDIO and USB.
It looks like the good old pre-PCI-2.2 days, where you had to have a
separate cable between your network card and the base-board for the
wake-up interrupt to be delivered. Starting with PCI-2.2, the bus can
carry the signal just fine. With PCIe, it should just be an interrupt
TLP sent to the RC, but that's obviously not within the capabilities of
the HW.
Anyway, it'd be good if the Marvell people could chime in and let us
know how they'd prefer to handle this.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* Re: [PATCH v1 iproute2-next 2/4] Sync up rdma_netlink.h
From: Steve Wise @ 2019-02-26 17:15 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: dsahern, stephen, netdev, linux-rdma
In-Reply-To: <20190223092820.GN23561@mtr-leonro.mtl.com>
On 2/23/2019 3:28 AM, Leon Romanovsky wrote:
> On Thu, Feb 21, 2019 at 08:19:07AM -0800, Steve Wise wrote:
>> Pull in the latest rdma_netlink.h to get the RDMA_NLDEV_CMD_NEWLINK /
>> RDMA_NLDEV_CMD_DELLINK API.
>>
>> Signed-off-by: Steve Wise <swise@opengridcomputing.com>
>> ---
>> rdma/include/uapi/rdma/rdma_netlink.h | 74 +++++++++++++++++++++++++++--------
>> 1 file changed, 58 insertions(+), 16 deletions(-)
>>
> If my series go first, this patch won't be needed.
>
> Thanks,
> Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
Some changes are still needed because your iproute2 series that was
recently merged didn't include the kernel NEWLINK/DELLINK changes. But
I've rebased this and all the IWPM changes are already there as part of
your series.
Thanks,
Steve.
^ permalink raw reply
* Re: [PATCH v1 iproute2-next 1/4] rdma: add helper rd_sendrecv_msg()
From: Steve Wise @ 2019-02-26 17:19 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: dsahern, stephen, netdev, linux-rdma
In-Reply-To: <20190223093122.GO23561@mtr-leonro.mtl.com>
On 2/23/2019 3:31 AM, Leon Romanovsky wrote:
> On Sat, Feb 23, 2019 at 11:26:15AM +0200, Leon Romanovsky wrote:
>> On Thu, Feb 21, 2019 at 08:19:03AM -0800, Steve Wise wrote:
>>> This function sends the constructed netlink message and then
>>> receives the response, displaying any error text.
>>>
>>> Change 'rdma dev set' to use it.
>>>
>>> Signed-off-by: Steve Wise <swise@opengridcomputing.com>
>>> ---
>>> rdma/dev.c | 2 +-
>>> rdma/rdma.h | 1 +
>>> rdma/utils.c | 21 +++++++++++++++++++++
>>> 3 files changed, 23 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/rdma/dev.c b/rdma/dev.c
>>> index 60ff4b31e320..d2949c378f08 100644
>>> --- a/rdma/dev.c
>>> +++ b/rdma/dev.c
>>> @@ -273,7 +273,7 @@ static int dev_set_name(struct rd *rd)
>>> mnl_attr_put_u32(rd->nlh, RDMA_NLDEV_ATTR_DEV_INDEX, rd->dev_idx);
>>> mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_DEV_NAME, rd_argv(rd));
>>>
>>> - return rd_send_msg(rd);
>>> + return rd_sendrecv_msg(rd, seq);
>>> }
>>>
>>> static int dev_one_set(struct rd *rd)
>>> diff --git a/rdma/rdma.h b/rdma/rdma.h
>>> index 547bb5749a39..20be2f12c4f8 100644
>>> --- a/rdma/rdma.h
>>> +++ b/rdma/rdma.h
>>> @@ -115,6 +115,7 @@ bool rd_check_is_key_exist(struct rd *rd, const char *key);
>>> */
>>> int rd_send_msg(struct rd *rd);
>>> int rd_recv_msg(struct rd *rd, mnl_cb_t callback, void *data, uint32_t seq);
>>> +int rd_sendrecv_msg(struct rd *rd, unsigned int seq);
>>> void rd_prepare_msg(struct rd *rd, uint32_t cmd, uint32_t *seq, uint16_t flags);
>>> int rd_dev_init_cb(const struct nlmsghdr *nlh, void *data);
>>> int rd_attr_cb(const struct nlattr *attr, void *data);
>>> diff --git a/rdma/utils.c b/rdma/utils.c
>>> index 069d44fece10..a6f2826c9605 100644
>>> --- a/rdma/utils.c
>>> +++ b/rdma/utils.c
>>> @@ -664,6 +664,27 @@ int rd_recv_msg(struct rd *rd, mnl_cb_t callback, void *data, unsigned int seq)
>>> return ret;
>>> }
>>>
>>> +static int null_cb(const struct nlmsghdr *nlh, void *data)
>>> +{
>>> + return MNL_CB_OK;
>>> +}
>>> +
>>> +int rd_sendrecv_msg(struct rd *rd, unsigned int seq)
>>> +{
>>> + int ret;
>>> +
>>> + ret = rd_send_msg(rd);
>>> + if (ret) {
>>> + perror(NULL);
>> This is more or less already done in rd_send_msg() and that function
>> prints something in case of execution error. So the missing piece
>> is to update rd_recv_msg(), so all places will "magically" print errors
>> and not only dev_set_name().
>>
>>> + goto out;
>>> + }
>>> + ret = rd_recv_msg(rd, null_cb, rd, seq);
> Will this "null_cb" work for all send/recv flows or only in flows where
> response can be error only?
Only those flows where no nl attributes are expected to be returned.
> Will we need this recv_msg if we implement
> extack support?
I'm not sure how extack works. Do you know?
Thanks!
Steve.
^ permalink raw reply
* Re: [PATCH net-next] net: sched: fix typo in walker_check_empty()
From: David Miller @ 2019-02-26 17:20 UTC (permalink / raw)
To: vladbu; +Cc: netdev, jhs, xiyou.wangcong, jiri
In-Reply-To: <20190226153440.17349-1-vladbu@mellanox.com>
From: Vlad Buslov <vladbu@mellanox.com>
Date: Tue, 26 Feb 2019 17:34:40 +0200
> Function walker_check_empty() incorrectly verifies that tp pointer is not
> NULL, instead of actual filter pointer. Fix conditional to check the right
> pointer. Adjust filter pointer naming accordingly to other cls API
> functions.
>
> Fixes: 6676d5e416ee ("net: sched: set dedicated tcf_walker flag when tp is empty")
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
> Reported-by: Cong Wang <xiyou.wangcong@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] tc-testing: gitignore, ignore local tdc config file
From: David Miller @ 2019-02-26 17:20 UTC (permalink / raw)
To: vladbu; +Cc: netdev, jhs, shuah, lucasb
In-Reply-To: <20190226153709.17587-1-vladbu@mellanox.com>
From: Vlad Buslov <vladbu@mellanox.com>
Date: Tue, 26 Feb 2019 17:37:09 +0200
> Comment in tdc_config.py recommends putting customizations in
> tdc_config_local.py file that wasn't included in gitignore. Add the local
> config file to gitignore.
>
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Applied.
^ permalink raw reply
* Re: [PATCH v1 iproute2-next 3/4] rdma: add 'link add/delete' commands
From: Steve Wise @ 2019-02-26 17:24 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: dsahern, stephen, netdev, linux-rdma
In-Reply-To: <20190223094311.GP23561@mtr-leonro.mtl.com>
On 2/23/2019 3:43 AM, Leon Romanovsky wrote:
> On Thu, Feb 21, 2019 at 08:19:12AM -0800, Steve Wise wrote:
>> Add new 'link' subcommand 'add' and 'delete' to allow binding a soft-rdma
>> device to a netdev interface.
>>
>> EG:
>>
>> rdma link add rxe_eth0 type rxe netdev eth0
>> rdma link delete rxe_eth0
>>
>> Signed-off-by: Steve Wise <swise@opengridcomputing.com>
>> ---
>> rdma/link.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> rdma/rdma.h | 1 +
>> rdma/utils.c | 2 +-
>> 3 files changed, 69 insertions(+), 1 deletion(-)
>>
>> diff --git a/rdma/link.c b/rdma/link.c
>> index c064be627be2..afaf19663728 100644
>> --- a/rdma/link.c
>> +++ b/rdma/link.c
>> @@ -14,6 +14,9 @@
>> static int link_help(struct rd *rd)
>> {
>> pr_out("Usage: %s link show [DEV/PORT_INDEX]\n", rd->filename);
>> + pr_out("Usage: %s link add NAME type TYPE netdev NETDEV\n",
>> + rd->filename);
>> + pr_out("Usage: %s link delete NAME\n", rd->filename);
>> return 0;
>> }
>>
>> @@ -341,10 +344,74 @@ static int link_show(struct rd *rd)
>> return rd_exec_link(rd, link_one_show, true);
>> }
>>
>> +static int link_add(struct rd *rd)
>> +{
>> + char *name;
>> + char *type = NULL;
>> + char *dev = NULL;
>> + uint32_t seq;
>> +
>> + if (rd_no_arg(rd)) {
>> + pr_err("No link name was supplied\n");
> I think that it is better to have instruction message and not error
> message: "Please provide ...".
Ok. Perhaps a new utility rd_exec_require_link() can be created?
>> + return -EINVAL;
>> + }
>> + name = rd_argv(rd);
>> + rd_arg_inc(rd);
>> + while (!rd_no_arg(rd)) {
>> + if (rd_argv_match(rd, "type")) {
>> + rd_arg_inc(rd);
>> + type = rd_argv(rd);
>> + } else if (rd_argv_match(rd, "netdev")) {
>> + rd_arg_inc(rd);
>> + dev = rd_argv(rd);
>> + } else {
>> + pr_err("Invalid parameter %s\n", rd_argv(rd));
>> + return -EINVAL;
>> + }
>> + rd_arg_inc(rd);
> Please use chains of struct rd_cmd and rd_exec_cmd() instead of
> open-coding parser.
Ok. Like your recently merged series did...
>> + }
>> + if (!type) {
>> + pr_err("No type was supplied\n");
>> + return -EINVAL;
> General parser handle it.
Ok.
>> + }
>> + if (!dev) {
>> + pr_err("No net device was supplied\n");
>> + return -EINVAL;
>> + }
> rd_exec_require_dev() ???
Looks like I can use that. I'll try it.
Thanks!
Steve.
^ permalink raw reply
* Re: [PATCH] bpf: decrease usercnt if bpf_map_new_fd() fails in bpf_map_get_fd_by_id()
From: Martin Lau @ 2019-02-26 17:33 UTC (permalink / raw)
To: zerons
Cc: ast@kernel.org, daniel@iogearbox.net, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1551190537-28694-1-git-send-email-sironhide0null@gmail.com>
On Tue, Feb 26, 2019 at 10:15:37PM +0800, zerons wrote:
> [ Upstream commit c91951f15978f1a0c6b65f063d30f7ea7bc6fb42 ]
>
> In bpf/syscall.c, bpf_map_get_fd_by_id() use bpf_map_inc_not_zero() to increase
> the refcount, both map->refcnt and map->usercnt. Then, if bpf_map_new_fd() fails,
> should handle map->usercnt too.
Good catch! Thanks for the fix.
Fixes: bd5f5f4ecb78 ("bpf: Add BPF_MAP_GET_FD_BY_ID")
Acked-by: Martin KaFai Lau <kafai@fb.com>
^ permalink raw reply
* Re: [PATCH net-next 1/2] vxlan: add extack support for create and changelink
From: Petr Machata @ 2019-02-26 17:45 UTC (permalink / raw)
To: Roopa Prabhu
Cc: davem@davemloft.net, netdev@vger.kernel.org,
dsa@cumulusnetworks.com, sd@queasysnail.net,
johannes@sipsolutions.net
In-Reply-To: <CAJieiUhKFrhoMcy-Bz1JmWEQ=8OooeZpb_sbD6zcKwotfJooBQ@mail.gmail.com>
Roopa Prabhu <roopa@cumulusnetworks.com> writes:
> On Tue, Feb 26, 2019 at 5:51 AM Petr Machata <petrm@mellanox.com> wrote:
>>
>> Many of the flags for which you call this don't actually have the else
>> branch. However I suspect there are no good reasons not to allow
>> resetting a flag.
>>
>> Reviewed-by: Petr Machata <petrm@mellanox.com>
>
> yes, correct, that was intentional.
> also, I am not sure what is the best way to support reseting of a NLA_FLAG.
> Absence of the flag attribute in the request cannot be the reason for
> resetting or clearing the flag.
Yeah, I was thinking about that. It looks like this would need a new set
of u8 attributes :-(
> None of the NLA_FLAG attributes support changing the flag today. This
> patch does not change that.
^ 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