* Re: [patch iproute2 rfc 1/2] devlink: introduce support for showing port flavours
From: Stephen Hemminger @ 2018-03-27 15:28 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, idosch, jakub.kicinski, mlxsw, andrew,
vivien.didelot, f.fainelli, michael.chan, ganeshgr, saeedm,
simon.horman, pieter.jansenvanvuuren, john.hurley,
dirk.vandermerwe, alexander.h.duyck, ogerlitz, dsahern,
vijaya.guvva, satananda.burla, raghu.vatsavayi, felix.manlunas,
gospo, sathya.perla, vasundhara-v.volam, tariqt, eranbe,
jeffrey.t.kirsher
In-Reply-To: <20180323063558.11718-1-jiri@resnulli.us>
On Fri, 23 Mar 2018 07:35:57 +0100
Jiri Pirko <jiri@resnulli.us> wrote:
> +static const char *port_flavour_name(uint16_t flavour)
> +{
> + switch (flavour) {
> + case DEVLINK_PORT_FLAVOUR_PHYSICAL: return "physical";
> + case DEVLINK_PORT_FLAVOUR_PF_REP: return "pf_rep";
> + case DEVLINK_PORT_FLAVOUR_VF_REP: return "vf_rep";
> + default: return "<unknown flavour>";
> + }
> +}
> +
need to break lines even in switch statement.
^ permalink raw reply
* Re: [PATCH 1/6] rhashtable: improve documentation for rhashtable_walk_peek()
From: Herbert Xu @ 2018-03-27 15:30 UTC (permalink / raw)
To: NeilBrown; +Cc: Thomas Graf, netdev, linux-kernel, Tom Herbert
In-Reply-To: <152210718418.11435.11573013181393548255.stgit@noble>
On Tue, Mar 27, 2018 at 10:33:04AM +1100, NeilBrown wrote:
> The documentation for rhashtable_walk_peek() wrong. It claims to
> return the *next* entry, whereas it in fact returns the *previous*
> entry.
> However if no entries have yet been returned - or if the iterator
> was reset due to a resize event, then rhashtable_walk_peek()
> *does* return the next entry, but also advances the iterator.
>
> I suspect that this interface should be discarded and the one user
> should be changed to not require it. Possibly this patch should be
> seen as a first step in that conversation.
>
> This patch mostly corrects the documentation, but does make a
> small code change so that the documentation can be correct without
> listing too many special cases. I don't think the one user will
> be affected by the code change.
>
> Signed-off-by: NeilBrown <neilb@suse.com>
We should cc Tom Herbert too since he wrote this code.
> ---
> lib/rhashtable.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/lib/rhashtable.c b/lib/rhashtable.c
> index 3825c30aaa36..24a57ca494cb 100644
> --- a/lib/rhashtable.c
> +++ b/lib/rhashtable.c
> @@ -853,13 +853,17 @@ void *rhashtable_walk_next(struct rhashtable_iter *iter)
> EXPORT_SYMBOL_GPL(rhashtable_walk_next);
>
> /**
> - * rhashtable_walk_peek - Return the next object but don't advance the iterator
> + * rhashtable_walk_peek - Return the previously returned object without advancing the iterator
> * @iter: Hash table iterator
> *
> - * Returns the next object or NULL when the end of the table is reached.
> + * Returns the last object returned, or NULL if no object has yet been returned.
> + * If the previously returned object has since been removed, then some other arbitrary
> + * object maybe returned, or possibly NULL will be returned. In that case, the
> + * iterator might be advanced.
> *
> * Returns -EAGAIN if resize event occurred. Note that the iterator
> - * will rewind back to the beginning and you may continue to use it.
> + * will rewind back to the beginning and rhashtable_walk_next() should be
> + * used to get the next object.
> */
> void *rhashtable_walk_peek(struct rhashtable_iter *iter)
> {
> @@ -880,7 +884,12 @@ void *rhashtable_walk_peek(struct rhashtable_iter *iter)
> * the table hasn't changed.
> */
> iter->skip--;
> - }
> + } else
> + /* ->skip is only zero after rhashtable_walk_start()
> + * or when the iterator is reset. In this case there
> + * is no previous object to return.
> + */
> + return NULL;
>
> return __rhashtable_walk_find_next(iter);
> }
>
>
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* RE: [PATCH net-next, 2/2] hv_netvsc: Add range checking for rx packet offset and length
From: Haiyang Zhang @ 2018-03-27 15:35 UTC (permalink / raw)
To: Stephen Hemminger
Cc: davem@davemloft.net, netdev@vger.kernel.org, olaf@aepfle.de,
Stephen Hemminger, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, vkuznets@redhat.com
In-Reply-To: <20180327082245.591612dc@xeon-e3>
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Tuesday, March 27, 2018 11:23 AM
> To: Haiyang Zhang <haiyangz@linuxonhyperv.com>
> Cc: Haiyang Zhang <haiyangz@microsoft.com>; davem@davemloft.net;
> netdev@vger.kernel.org; olaf@aepfle.de; Stephen Hemminger
> <sthemmin@microsoft.com>; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; vkuznets@redhat.com
> Subject: Re: [PATCH net-next, 2/2] hv_netvsc: Add range checking for rx packet
> offset and length
>
> On Thu, 22 Mar 2018 12:01:14 -0700
> Haiyang Zhang <haiyangz@linuxonhyperv.com> wrote:
>
> > From: Haiyang Zhang <haiyangz@microsoft.com>
> >
> > This patch adds range checking for rx packet offset and length.
> > It may only happen if there is a host side bug.
> >
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > ---
> > drivers/net/hyperv/hyperv_net.h | 1 +
> > drivers/net/hyperv/netvsc.c | 17 +++++++++++++++--
> > 2 files changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/hyperv/hyperv_net.h
> > b/drivers/net/hyperv/hyperv_net.h index 0db3bd1ea06f..49c05ac894e5
> > 100644
> > --- a/drivers/net/hyperv/hyperv_net.h
> > +++ b/drivers/net/hyperv/hyperv_net.h
> > @@ -793,6 +793,7 @@ struct netvsc_device {
> >
> > /* Receive buffer allocated by us but manages by NetVSP */
> > void *recv_buf;
> > + u32 recv_buf_size; /* allocated bytes */
> > u32 recv_buf_gpadl_handle;
> > u32 recv_section_cnt;
> > u32 recv_section_size;
> > diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> > index 1ddb2c39b6e4..a6700d65f206 100644
> > --- a/drivers/net/hyperv/netvsc.c
> > +++ b/drivers/net/hyperv/netvsc.c
> > @@ -289,6 +289,8 @@ static int netvsc_init_buf(struct hv_device *device,
> > goto cleanup;
> > }
> >
> > + net_device->recv_buf_size = buf_size;
> > +
> > /*
> > * Establish the gpadl handle for this buffer on this
> > * channel. Note: This call uses the vmbus connection rather @@
> > -1095,11 +1097,22 @@ static int netvsc_receive(struct net_device
> > *ndev,
> >
> > /* Each range represents 1 RNDIS pkt that contains 1 ethernet frame */
> > for (i = 0; i < count; i++) {
> > - void *data = recv_buf
> > - + vmxferpage_packet->ranges[i].byte_offset;
> > + u32 offset = vmxferpage_packet->ranges[i].byte_offset;
> > u32 buflen = vmxferpage_packet->ranges[i].byte_count;
> > + void *data;
> > int ret;
> >
> > + if (unlikely(offset + buflen > net_device->recv_buf_size)) {
> > + status = NVSP_STAT_FAIL;
> > + netif_err(net_device_ctx, rx_err, ndev,
> > + "Packet offset:%u + len:%u too big\n",
> > + offset, buflen);
> > +
> > + continue;
> > + }
> > +
>
> If one part of the RNDIS packet is wrong then the whole receive buffer is
> damaged. Just return, don't continue.
>
> It could really just be a statistic and a one shot log message.
I will let the loop terminates and send NVSP status fail to the host.
For statistics, this range check is to catch potential host side issues, just like
these checks in the same function earlier:
/* Make sure this is a valid nvsp packet */
if (unlikely(nvsp->hdr.msg_type != NVSP_MSG1_TYPE_SEND_RNDIS_PKT)) {
netif_err(net_device_ctx, rx_err, ndev,
"Unknown nvsp packet type received %u\n",
nvsp->hdr.msg_type);
return 0;
}
if (unlikely(vmxferpage_packet->xfer_pageset_id != NETVSC_RECEIVE_BUFFER_ID)) {
netif_err(net_device_ctx, rx_err, ndev,
"Invalid xfer page set id - expecting %x got %x\n",
NETVSC_RECEIVE_BUFFER_ID,
vmxferpage_packet->xfer_pageset_id);
return 0;
}
If these kinds of errors need statistics, there will be many stat variables... Maybe we
should just create one stat variable for all of the "invalid format from host"?
Thanks,
- Haiyang
^ permalink raw reply
* Re: [PATCH 5/6] net/ipv4: Update ip_tunnel_metadata_cnt static key to modern api
From: Davidlohr Bueso @ 2018-03-27 15:37 UTC (permalink / raw)
To: David Miller; +Cc: peterz, mingo, akpm, linux-kernel, netdev, dbueso
In-Reply-To: <20180327.114402.619275306009710582.davem@davemloft.net>
On Tue, 27 Mar 2018, David Miller wrote:
>From: Davidlohr Bueso <dave@stgolabs.net>
>Date: Mon, 26 Mar 2018 14:09:28 -0700
>
>> No changes in refcount semantics -- key init is false; replace
>>
>> static_key_slow_inc|dec with static_branch_inc|dec
>> static_key_false with static_branch_unlikely
>>
>> Added a '_key' suffix to i2c_trace_msg, for better self
>> documentation.
>
>I see no reference to i2c_trace_msg in this patch.
Sorry, that was from a previous changelog. The reason why it was not updated was
because I chose not to rename to ip_tunnel_metadata_cnt_key, so the whole sentence
needed removal. Do you want a v3 or can you pick up as is?
Thanks,
Davidlohr
^ permalink raw reply
* [bpf-next PATCH] net: br_vlan build error
From: John Fastabend @ 2018-03-27 15:38 UTC (permalink / raw)
To: stephen, 3chas3; +Cc: netdev, davem
Fix build error in br_if.c
net/bridge/br_if.c: In function ‘br_mtu’:
net/bridge/br_if.c:458:8: error: ‘const struct net_bridge’ has no member named ‘vlan_enabled’
if (br->vlan_enabled)
^
net/bridge/br_if.c:462:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
Fixes: 419d14af9e07f ("bridge: Allow max MTU when multiple VLANs present")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
net/bridge/br_if.c | 27 +++++++++++++++++----------
1 file changed, 17 insertions(+), 10 deletions(-)
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 48dc4d2..2262424 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -424,16 +424,6 @@ int br_del_bridge(struct net *net, const char *name)
return ret;
}
-static bool min_mtu(int a, int b)
-{
- return a < b ? 1 : 0;
-}
-
-static bool max_mtu(int a, int b)
-{
- return a > b ? 1 : 0;
-}
-
/* MTU of the bridge pseudo-device: ETH_DATA_LEN or the minimum of the ports */
static int __br_mtu(const struct net_bridge *br, bool (compare_fn)(int, int))
{
@@ -453,6 +443,17 @@ static int __br_mtu(const struct net_bridge *br, bool (compare_fn)(int, int))
return mtu;
}
+static bool min_mtu(int a, int b)
+{
+ return a < b ? 1 : 0;
+}
+
+#ifdef CONFIG_BRIDGE_VLAN_FILTERING
+static bool max_mtu(int a, int b)
+{
+ return a > b ? 1 : 0;
+}
+
int br_mtu(const struct net_bridge *br)
{
if (br->vlan_enabled)
@@ -460,6 +461,12 @@ int br_mtu(const struct net_bridge *br)
else
return __br_mtu(br, min_mtu);
}
+#else
+int br_mtu(const struct net_bridge *br)
+{
+ return __br_mtu(br, min_mtu);
+}
+#endif
static void br_set_gso_limits(struct net_bridge *br)
{
^ permalink raw reply related
* Re: [pull request][net V2 0/8] Mellanox, mlx5 fixes 2018-03-23
From: David Miller @ 2018-03-27 15:42 UTC (permalink / raw)
To: saeedm; +Cc: netdev
In-Reply-To: <20180326210505.11007-1-saeedm@mellanox.com>
From: Saeed Mahameed <saeedm@mellanox.com>
Date: Mon, 26 Mar 2018 14:04:57 -0700
> The following series includes fixes for mlx5 netdev and eswitch.
>
> v1->v2:
> - Fixed commit message quotation marks in patch #7
Pulled.
> For -stable v4.12
> ('net/mlx5e: Avoid using the ipv6 stub in the TC offload neigh update path')
> ('net/mlx5e: Fix traffic being dropped on VF representor')
>
> For -stable v4.13
> ('net/mlx5e: Fix memory usage issues in offloading TC flows')
> ('net/mlx5e: Verify coalescing parameters in range')
>
> For -stable v4.14
> ('net/mlx5e: Don't override vport admin link state in switchdev mode')
>
> For -stable v4.15
> ('108b2b6d5c02 net/mlx5e: Sync netdev vxlan ports at open')
Queued up for -stable.
Thanks.
^ permalink raw reply
* Re: [B.A.T.M.A.N.] [PATCH 03/17] batman-adv: Add network_coding and mcast sysfs files to README
From: Linus Lüssing @ 2018-03-27 15:43 UTC (permalink / raw)
To: Jiri Pirko
Cc: The list for a Better Approach To Mobile Ad-hoc Networking,
Sven Eckelmann, netdev, davem
In-Reply-To: <20161029105628.GE1692@nanopsycho.orion>
On Sat, Oct 29, 2016 at 12:56:28PM +0200, Jiri Pirko wrote:
> >> I strongly believe it is a huge mistake to use sysfs for things like
> >> this. This should be done via generic netlink api.
> >
> >This doesn't change the problem that it is already that way. This patch
> >only adds the list of available files to the README.
>
> Sure. Just found out you did it like that. Therefore I commented. I
> suggest to rework the api to use genl entirely.
Hi Jiri,
Thanks for sharing your thoughts!
Could you explain a bit more on which disadvantages you see in
the usage of sysfs here?
Regards, Linus
^ permalink raw reply
* Re: [PATCH 5/6] net/ipv4: Update ip_tunnel_metadata_cnt static key to modern api
From: David Miller @ 2018-03-27 15:44 UTC (permalink / raw)
To: dave; +Cc: peterz, mingo, akpm, linux-kernel, netdev, dbueso
In-Reply-To: <20180326210929.5244-6-dave@stgolabs.net>
From: Davidlohr Bueso <dave@stgolabs.net>
Date: Mon, 26 Mar 2018 14:09:28 -0700
> No changes in refcount semantics -- key init is false; replace
>
> static_key_slow_inc|dec with static_branch_inc|dec
> static_key_false with static_branch_unlikely
>
> Added a '_key' suffix to i2c_trace_msg, for better self
> documentation.
I see no reference to i2c_trace_msg in this patch.
^ permalink raw reply
* RE: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information
From: Steve Wise @ 2018-03-27 15:45 UTC (permalink / raw)
To: 'Jason Gunthorpe', 'Leon Romanovsky'
Cc: 'David Ahern', stephen, netdev, linux-rdma
In-Reply-To: <20180327152356.GH12318@ziepe.ca>
>
> On Tue, Mar 27, 2018 at 06:15:44PM +0300, Leon Romanovsky wrote:
> > On Tue, Mar 27, 2018 at 08:44:55AM -0600, Jason Gunthorpe wrote:
> > > On Tue, Mar 27, 2018 at 06:21:41AM +0300, Leon Romanovsky wrote:
> > > > On Mon, Mar 26, 2018 at 04:30:33PM -0600, Jason Gunthorpe wrote:
> > > > > On Mon, Mar 26, 2018 at 04:34:44PM -0500, Steve Wise wrote:
> > > > > >
> > > > > > On 3/26/2018 4:15 PM, Jason Gunthorpe wrote:
> > > > > > > On Mon, Mar 26, 2018 at 09:30:41AM -0500, Steve Wise wrote:
> > > > > > >>
> > > > > > >> On 3/26/2018 9:17 AM, David Ahern wrote:
> > > > > > >>> On 2/27/18 9:07 AM, Steve Wise wrote:
> > > > > > >>>> diff --git a/rdma/rdma.h b/rdma/rdma.h
> > > > > > >>>> index 5809f70..e55205b 100644
> > > > > > >>>> +++ b/rdma/rdma.h
> > > > > > >>>> @@ -18,10 +18,12 @@
> > > > > > >>>> #include <libmnl/libmnl.h>
> > > > > > >>>> #include <rdma/rdma_netlink.h>
> > > > > > >>>> #include <time.h>
> > > > > > >>>> +#include <net/if_arp.h>
> > > > > > >>>>
> > > > > > >>>> #include "list.h"
> > > > > > >>>> #include "utils.h"
> > > > > > >>>> #include "json_writer.h"
> > > > > > >>>> +#include <rdma/rdma_cma.h>
> > > > > > >>>>
> > > > > > >>> did you forget to add rdma_cma.h? I don't see that file in my
> repo.
> > > > > > >> It is provided by the rdma-core package, upon which rdma tool
> now
> > > > > > >> depends for the rdma_port_space enum.
> > > > > > > It is a kernel bug that enum is not in an include/uapi/rdma
> header
> > > > > > >
> > > > > > > Fix it there and don't try to use rdma-core headers to get kernel
> ABI.
> > > > > > >
> > > > > > > Jason
> > > > > >
> > > > > > I wish you'd commented on this just a little sooner. I just resent
> v3
> > > > > > of this series... with rdma_cma.h included. :)
> > > > > >
> > > > > > How about the restrack/nldev code just translates the port space
> from
> > > > > > enum rdma_port_space to a new ABI enum, say
> nldev_rdma_port_space, that
> > > > > > i add to rdma_netlink.h? I'd hate to open the can of worms of
> trying to
> > > > > > split rdma_cma.h into uabi and no uabi headers. :(
> > > > >
> > > > > If port space is already part of the ABI there isn't much reason to
> > > > > translate it.
> > > > >
> > > > > You just need to pick the right header to put it in, since it is a verbs
> > > > > define it doesn't belong in the netlink header.
> > > >
> > > > I completely understand Steve's concerns.
> > > >
> > > > I tried to do such thing (expose kernel headers) in first incarnation of
> > > > rdmatool with attempt to clean IB/core as well to ensure that we
> won't expose
> > > > anything that is not implemented. It didn't go well.
> > >
> > > rdma-core is now using the kernel uapi/ headers natively, seems to be
> > > going OK. What problem did you face?
> >
> > I didn't agree to move to UAPI defines which are not implemented and
> not
> > used in the kernel, so I sent small number of patches similar to those [1,
> 2].
> >
> > Those patches were rejected.
> >
> > So please don't mix Steve's need to use 3 defines with very large and
> > painful task to expose proper UAPIs.
>
> Steve can just move the 3 defines he needs to the uapi, we are doing
> this incrementally..
>
> rdma_core does not define kernel ABI and it is totally wrong to use
> random constants from rdma_cma.h as kernel ABI.
>
Proposal:
Since the cm_id port space is part of the rdma_ucm_create_id struct in include/uapi/rdma/rdma_user_cm.h, I'll move the rdma_port_space enum there. And then my iproute2 series will have to add a copy of rdma_user_cm.h locally into rdma/include/uapi/rdma, right?
Will that work for everyone?
Steve.
^ permalink raw reply
* Re: [PATCH 1/6] rhashtable: improve documentation for rhashtable_walk_peek()
From: David Miller @ 2018-03-27 15:47 UTC (permalink / raw)
To: neilb; +Cc: tgraf, herbert, netdev, linux-kernel
In-Reply-To: <152210718418.11435.11573013181393548255.stgit@noble>
From: NeilBrown <neilb@suse.com>
Date: Tue, 27 Mar 2018 10:33:04 +1100
> The documentation for rhashtable_walk_peek() wrong. It claims to
> return the *next* entry, whereas it in fact returns the *previous*
> entry.
> However if no entries have yet been returned - or if the iterator
> was reset due to a resize event, then rhashtable_walk_peek()
> *does* return the next entry, but also advances the iterator.
>
> I suspect that this interface should be discarded and the one user
> should be changed to not require it. Possibly this patch should be
> seen as a first step in that conversation.
>
> This patch mostly corrects the documentation, but does make a
> small code change so that the documentation can be correct without
> listing too many special cases. I don't think the one user will
> be affected by the code change.
>
> Signed-off-by: NeilBrown <neilb@suse.com>
Please mention the "one user" explicitly in both locations where
you refer to it in this commit message.
Thank you.
^ permalink raw reply
* Re: [PATCH 3/6] rhashtable: reset intr when rhashtable_walk_start sees new table
From: Herbert Xu @ 2018-03-27 15:47 UTC (permalink / raw)
To: NeilBrown; +Cc: Thomas Graf, netdev, linux-kernel
In-Reply-To: <152210718427.11435.17689950338335773862.stgit@noble>
On Tue, Mar 27, 2018 at 10:33:04AM +1100, NeilBrown wrote:
> The documentation claims that when rhashtable_walk_start_check()
> detects a resize event, it will rewind back to the beginning
> of the table. This is not true. We need to set ->slot and
> ->skip to be zero for it to be true.
>
> Signed-off-by: NeilBrown <neilb@suse.com>
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [bpf-next PATCH] net: br_vlan build error
From: Nikolay Aleksandrov @ 2018-03-27 15:49 UTC (permalink / raw)
To: John Fastabend, stephen, 3chas3; +Cc: netdev, davem
In-Reply-To: <20180327153859.28865.74511.stgit@john-Precision-Tower-5810>
On 27/03/18 18:38, John Fastabend wrote:
> Fix build error in br_if.c
>
> net/bridge/br_if.c: In function ‘br_mtu’:
> net/bridge/br_if.c:458:8: error: ‘const struct net_bridge’ has no member named ‘vlan_enabled’
> if (br->vlan_enabled)
> ^
> net/bridge/br_if.c:462:1: warning: control reaches end of non-void function [-Wreturn-type]
> }
> ^
> Fixes: 419d14af9e07f ("bridge: Allow max MTU when multiple VLANs present")
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
> net/bridge/br_if.c | 27 +++++++++++++++++----------
> 1 file changed, 17 insertions(+), 10 deletions(-)
>
I'm not sure what the rules about merging are, but just in case I
already fixed this in net-next couple of days ago.
commit 82792a070b16
Author: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Fri Mar 23 18:27:06 2018 +0200
net: bridge: fix direct access to bridge vlan_enabled and use helper
https://patchwork.ozlabs.org/patch/890043/
Cheers,
Nik
^ permalink raw reply
* Re: [PATCH 4/6] rhashtable: allow a walk of the hash table without missing objects.
From: David Miller @ 2018-03-27 15:49 UTC (permalink / raw)
To: neilb; +Cc: tgraf, herbert, netdev, linux-kernel
In-Reply-To: <152210718430.11435.5761213978298714527.stgit@noble>
From: NeilBrown <neilb@suse.com>
Date: Tue, 27 Mar 2018 10:33:04 +1100
> In many cases where the walker needs to drop out of RCU protection,
> it will take a reference to the object and this can prevent it from
> being removed from the hash table. In those cases, the last-returned
> object can still be used as a cursor. rhashtable cannot detect
> these cases itself.
Merely having an elevated reference count does not explicitly prevent
the object from being removed from the hash table.
This invariant might hold for the particular user of the rhashtable
instance, but it is not always the case.
^ permalink raw reply
* Re: [PATCH 4/6] rhashtable: allow a walk of the hash table without missing objects.
From: Herbert Xu @ 2018-03-27 15:51 UTC (permalink / raw)
To: NeilBrown; +Cc: Thomas Graf, netdev, linux-kernel
In-Reply-To: <152210718430.11435.5761213978298714527.stgit@noble>
On Tue, Mar 27, 2018 at 10:33:04AM +1100, NeilBrown wrote:
>
> -int rhashtable_walk_start_check(struct rhashtable_iter *iter)
> +int rhashtable_walk_start_continue(struct rhashtable_iter *iter, struct rhash_head *obj)
> __acquires(RCU)
> {
> struct rhashtable *ht = iter->ht;
>
> rcu_read_lock();
>
> + if (!obj || iter->p != obj)
> + iter->p = NULL;
Why bother with this check at all? Couldn't we make it so that
if you call continue then you continue with the cursor otherwise
you set it to NULL as we currently do.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH 4/6] rhashtable: allow a walk of the hash table without missing objects.
From: Herbert Xu @ 2018-03-27 15:54 UTC (permalink / raw)
To: David Miller; +Cc: neilb, tgraf, netdev, linux-kernel
In-Reply-To: <20180327.114941.997071660018188736.davem@davemloft.net>
On Tue, Mar 27, 2018 at 11:49:41AM -0400, David Miller wrote:
>
> Merely having an elevated reference count does not explicitly prevent
> the object from being removed from the hash table.
>
> This invariant might hold for the particular user of the rhashtable
> instance, but it is not always the case.
I think having a new interface like this should work as if the
user knows that the element has not been removed from the hash
table then it should be safe to continue from it.
Of course people may misuse it but even using the current interface
correctly may result in lost objects due to concurrent removals.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v6 bpf-next 07/11] tracepoint: introduce kernel_tracepoint_find_by_name
From: Alexei Starovoitov @ 2018-03-27 15:53 UTC (permalink / raw)
To: Steven Rostedt, Mathieu Desnoyers
Cc: David S. Miller, Daniel Borkmann, Linus Torvalds, Peter Zijlstra,
netdev, kernel-team, linux-api
In-Reply-To: <20180327104242.3594bcac@gandalf.local.home>
On 3/27/18 7:42 AM, Steven Rostedt wrote:
> On Tue, 27 Mar 2018 10:18:24 -0400 (EDT)
> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
>
>> ----- On Mar 27, 2018, at 10:07 AM, rostedt rostedt@goodmis.org wrote:
>>
>>> On Mon, 26 Mar 2018 19:47:02 -0700
>>> Alexei Starovoitov <ast@fb.com> wrote:
>>>
>>>> From: Alexei Starovoitov <ast@kernel.org>
>>>>
>>>> introduce kernel_tracepoint_find_by_name() helper to let bpf core
>>>> find tracepoint by name and later attach bpf probe to a tracepoint
>>>>
>>>> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>>>
>>> Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
>>
>> Steven showed preference for tracepoint_kernel_find_by_name() at some
>> point (starting with a tracepoint_ prefix). I'm find with either of
>> the names.
>
> Yeah, I do prefer tracepoint_kernel_find_by_name() to stay consistent
> with the other tracepoint functions. But we have
> "for_each_kernel_tracepoint()" and not "for_each_tracepoint_kernel()",
> thus we need to pick being consistent with one or the other. One answer
> is to use tracpoint_kernel_find_by_name() and rename the for_each to
> for_each_tracpoint_kernel().
yep. that's exactly the reason I picked kernel_tracepoint_find_by_name()
to match for_each_kernel_tracepoint() naming.
I can certainly send a follow up patch to rename both to
*tracepoint_kernel* and then you can nack it because it breaks lttng :)
but let's do it in a separate thread.
Daniel,
do you mind adding { } as Steven requested while applying or
you want me to resubmit the whole thing?
Thanks!
^ permalink raw reply
* Re: [PATCH 5/6] rhashtable: support guaranteed successful insertion.
From: Herbert Xu @ 2018-03-27 15:56 UTC (permalink / raw)
To: NeilBrown; +Cc: Thomas Graf, netdev, linux-kernel
In-Reply-To: <152210718434.11435.6551477417902631683.stgit@noble>
On Tue, Mar 27, 2018 at 10:33:04AM +1100, NeilBrown wrote:
> The current rhashtable will fail an insertion if the hashtable
> it "too full", one of:
> - table already has 2^31 elements (-E2BIG)
> - a max_size was specified and table already has that
> many elements (rounded up to power of 2) (-E2BIG)
> - a single chain has more than 16 elements (-EBUSY)
> - table has more elements than the current table size,
> and allocating a new table fails (-ENOMEM)
> - a new page needed to be allocated for a nested table,
> and the memory allocation failed (-ENOMEM).
>
> A traditional hash table does not have a concept of "too full", and
> insertion only fails if the key already exists. Many users of hash
> tables have separate means of limiting the total number of entries,
> and are not susceptible to an attack which could cause unusually large
> hash chains. For those users, the need to check for errors when
> inserting objects to an rhashtable is an unnecessary burden and hence
> a potential source of bugs (as these failures are likely to be rare).
Did you actually encounter an insertion failure? The current code
should never fail an insertion until you actually run ouf memory.
That is unless you're using rhashtable when you should be using
rhlist instead.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [bpf-next PATCH] net: br_vlan build error
From: John Fastabend @ 2018-03-27 15:57 UTC (permalink / raw)
To: stephen, 3chas3; +Cc: netdev, davem
In-Reply-To: <20180327153859.28865.74511.stgit@john-Precision-Tower-5810>
On 03/27/2018 08:38 AM, John Fastabend wrote:
> Fix build error in br_if.c
>
> net/bridge/br_if.c: In function ‘br_mtu’:
> net/bridge/br_if.c:458:8: error: ‘const struct net_bridge’ has no member named ‘vlan_enabled’
> if (br->vlan_enabled)
> ^
> net/bridge/br_if.c:462:1: warning: control reaches end of non-void function [-Wreturn-type]
> }
> ^
> Fixes: 419d14af9e07f ("bridge: Allow max MTU when multiple VLANs present")
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
This is against net-next sorry for any confusion. Let me know
if you need me to respin.
@Chas, also I only build tested this to get my kernels building
again. You should probably verify I implemented what you expect
in the case without vlan filtering. It seems correct to me but
I'm not the expert here.
Thanks,
John
^ permalink raw reply
* Re: [Patch net] llc: properly handle dev_queue_xmit() return value
From: David Miller @ 2018-03-27 15:57 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, noamr
In-Reply-To: <20180326220833.17351-1-xiyou.wangcong@gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Mon, 26 Mar 2018 15:08:33 -0700
> llc_conn_send_pdu() pushes the skb into write queue and
> calls llc_conn_send_pdus() to flush them out. However, the
> status of dev_queue_xmit() is not returned to caller,
> in this case, llc_conn_state_process().
>
> llc_conn_state_process() needs hold the skb no matter
> success or failure, because it still uses it after that,
> therefore we should hold skb before dev_queue_xmit() when
> that skb is the one being processed by llc_conn_state_process().
>
> For other callers, they can just pass NULL and ignore
> the return value as they are.
>
> Reported-by: Noam Rathaus <noamr@beyondsecurity.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Applied, thanks Cong.
^ permalink raw reply
* Re: [bpf-next PATCH] net: br_vlan build error
From: John Fastabend @ 2018-03-27 15:58 UTC (permalink / raw)
To: Nikolay Aleksandrov, stephen, 3chas3; +Cc: netdev, davem
In-Reply-To: <f49dbcf5-7ba2-9225-603c-ca2ad95de6bd@cumulusnetworks.com>
On 03/27/2018 08:49 AM, Nikolay Aleksandrov wrote:
> On 27/03/18 18:38, John Fastabend wrote:
>> Fix build error in br_if.c
>>
>> net/bridge/br_if.c: In function ‘br_mtu’:
>> net/bridge/br_if.c:458:8: error: ‘const struct net_bridge’ has no member named ‘vlan_enabled’
>> if (br->vlan_enabled)
>> ^
>> net/bridge/br_if.c:462:1: warning: control reaches end of non-void function [-Wreturn-type]
>> }
>> ^
>> Fixes: 419d14af9e07f ("bridge: Allow max MTU when multiple VLANs present")
>> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
>> ---
>> net/bridge/br_if.c | 27 +++++++++++++++++----------
>> 1 file changed, 17 insertions(+), 10 deletions(-)
>>
>
> I'm not sure what the rules about merging are, but just in case I
> already fixed this in net-next couple of days ago.
>
> commit 82792a070b16
> Author: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> Date: Fri Mar 23 18:27:06 2018 +0200
>
> net: bridge: fix direct access to bridge vlan_enabled and use helper
>
> https://patchwork.ozlabs.org/patch/890043/
>
>
> Cheers,
> Nik
>
Ah dang sorry about the noise then drop my patch.
^ permalink raw reply
* Re: [PATCH net-next v2 0/2] kernel: add support to collect hardware logs in crash recovery kernel
From: Eric W. Biederman @ 2018-03-27 15:59 UTC (permalink / raw)
To: Rahul Lakkireddy
Cc: netdev@vger.kernel.org, linux-fsdevel@vger.kernel.org,
kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
davem@davemloft.net, viro@zeniv.linux.org.uk,
stephen@networkplumber.org, akpm@linux-foundation.org,
torvalds@linux-foundation.org, Ganesh GR, Nirranjan Kirubaharan,
Indranil Choudhury
In-Reply-To: <20180327152715.GA18097@chelsio.com>
Rahul Lakkireddy <rahul.lakkireddy@chelsio.com> writes:
> On Tuesday, March 03/27/18, 2018 at 18:47:34 +0530, Eric W. Biederman wrote:
>> Rahul Lakkireddy <rahul.lakkireddy@chelsio.com> writes:
>>
>> > On Saturday, March 03/24/18, 2018 at 20:50:52 +0530, Eric W. Biederman wrote:
>> >>
>> >> Rahul Lakkireddy <rahul.lakkireddy@chelsio.com> writes:
>> >>
>> >> > On production servers running variety of workloads over time, kernel
>> >> > panic can happen sporadically after days or even months. It is
>> >> > important to collect as much debug logs as possible to root cause
>> >> > and fix the problem, that may not be easy to reproduce. Snapshot of
>> >> > underlying hardware/firmware state (like register dump, firmware
>> >> > logs, adapter memory, etc.), at the time of kernel panic will be very
>> >> > helpful while debugging the culprit device driver.
>> >> >
>> >> > This series of patches add new generic framework that enable device
>> >> > drivers to collect device specific snapshot of the hardware/firmware
>> >> > state of the underlying device in the crash recovery kernel. In crash
>> >> > recovery kernel, the collected logs are exposed via /sys/kernel/crashdd/
>> >> > directory, which is copied by user space scripts for post-analysis.
>> >> >
>> >> > A kernel module crashdd is newly added. In crash recovery kernel,
>> >> > crashdd exposes /sys/kernel/crashdd/ directory containing device
>> >> > specific hardware/firmware logs.
>> >>
>> >> Have you looked at instead of adding a sysfs file adding the dumps
>> >> as additional elf notes in /proc/vmcore?
>> >>
>> >
>> > I see the crash recovery kernel's memory is not present in any of the
>> > the PT_LOAD headers. So, makedumpfile is not collecting the dumps
>> > that are in crash recovery kernel's memory.
>> >
>> > Also, are you suggesting exporting the dumps themselves as PT_NOTE
>> > instead? I'll look into doing it this way.
>>
>> Yes. I was suggesting exporting the dumps themselves as PT_NOTE
>> in /proc/vmcore. I think that will allow makedumpfile to collect
>> your new information without modification.
>>
>
> If I export the dumps themselves as PT_NOTE in /proc/vmcore, can the
> crash tool work without modification; i.e can crash tool extract these
> notes?
I believe crash would need to be taught about these notes. This is
something new.
However "readelf -a random_elf_file" does display elf notes, and elf
notes in general are not hard to extract.
What I expect from an enconding in ELF core dump format is a way to
captuer the data, a way to encode the data, and a way to transport the
data to the people who care. Analysis tools are easy enough after the
fact.
Eric
^ permalink raw reply
* Re: [PATCH net 1/1] net/smc: use announced length in sock_recvmsg()
From: David Miller @ 2018-03-27 16:00 UTC (permalink / raw)
To: ubraun; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl
In-Reply-To: <20180327084350.92847-2-ubraun@linux.vnet.ibm.com>
From: Ursula Braun <ubraun@linux.vnet.ibm.com>
Date: Tue, 27 Mar 2018 10:43:50 +0200
> Not every CLC proposal message needs the maximum buffer length.
> Due to the MSG_WAITALL flag, it is important to use the peeked
> real length when receiving the message.
>
> Fixes: d63d271ce2b5ce ("smc: switch to sock_recvmsg()")
> Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH iproute2] Revert "iproute: "list/flush/save default" selected all of the routes"
From: Stephen Hemminger @ 2018-03-27 16:01 UTC (permalink / raw)
To: Alexander Zubkov; +Cc: Serhey Popovych, Luca Boccassi, netdev@vger.kernel.org
In-Reply-To: <2ee0179a-79c8-8504-afe1-6310ed4e858b@msu.ru>
On Wed, 14 Mar 2018 21:26:40 +0100
Alexander Zubkov <green@msu.ru> wrote:
> Hello,
>
> For example, it can be fixed in such way (patch is below):
> - split handling of default and all/any
> - set needed attributes in get_addr: PREFIXLEN_SPECIFIED flag for default
> - and AF_UNSPEC for all/any
> In this case "ip route show default" shows only default route and "ip
> route show all" shows all routes. And both also work when family (-4 or
> -6) is specified.
> Serhey, does it goes in line with what you wanted to achieve? Because I
> do not know - may be there are reasons why all/any should be provided
> with specific family. If you think this solution is suitable, I'll do
> some additional tests and package the patch in a proper way for this
> mailing list.
> And I'm unsure if check for AF_DECnet and AF_MPLS should be kept in both
> branches. May be someone have some additional thoughts on that?
>
I applied this to master.
We can work on the other cases after that.
^ permalink raw reply
* Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information
From: Leon Romanovsky @ 2018-03-27 16:01 UTC (permalink / raw)
To: Steve Wise
Cc: 'Jason Gunthorpe', 'David Ahern', stephen, netdev,
linux-rdma
In-Reply-To: <023301d3c5e2$a2a48c90$e7eda5b0$@opengridcomputing.com>
[-- Attachment #1: Type: text/plain, Size: 3987 bytes --]
On Tue, Mar 27, 2018 at 10:45:30AM -0500, Steve Wise wrote:
>
> >
> > On Tue, Mar 27, 2018 at 06:15:44PM +0300, Leon Romanovsky wrote:
> > > On Tue, Mar 27, 2018 at 08:44:55AM -0600, Jason Gunthorpe wrote:
> > > > On Tue, Mar 27, 2018 at 06:21:41AM +0300, Leon Romanovsky wrote:
> > > > > On Mon, Mar 26, 2018 at 04:30:33PM -0600, Jason Gunthorpe wrote:
> > > > > > On Mon, Mar 26, 2018 at 04:34:44PM -0500, Steve Wise wrote:
> > > > > > >
> > > > > > > On 3/26/2018 4:15 PM, Jason Gunthorpe wrote:
> > > > > > > > On Mon, Mar 26, 2018 at 09:30:41AM -0500, Steve Wise wrote:
> > > > > > > >>
> > > > > > > >> On 3/26/2018 9:17 AM, David Ahern wrote:
> > > > > > > >>> On 2/27/18 9:07 AM, Steve Wise wrote:
> > > > > > > >>>> diff --git a/rdma/rdma.h b/rdma/rdma.h
> > > > > > > >>>> index 5809f70..e55205b 100644
> > > > > > > >>>> +++ b/rdma/rdma.h
> > > > > > > >>>> @@ -18,10 +18,12 @@
> > > > > > > >>>> #include <libmnl/libmnl.h>
> > > > > > > >>>> #include <rdma/rdma_netlink.h>
> > > > > > > >>>> #include <time.h>
> > > > > > > >>>> +#include <net/if_arp.h>
> > > > > > > >>>>
> > > > > > > >>>> #include "list.h"
> > > > > > > >>>> #include "utils.h"
> > > > > > > >>>> #include "json_writer.h"
> > > > > > > >>>> +#include <rdma/rdma_cma.h>
> > > > > > > >>>>
> > > > > > > >>> did you forget to add rdma_cma.h? I don't see that file in my
> > repo.
> > > > > > > >> It is provided by the rdma-core package, upon which rdma tool
> > now
> > > > > > > >> depends for the rdma_port_space enum.
> > > > > > > > It is a kernel bug that enum is not in an include/uapi/rdma
> > header
> > > > > > > >
> > > > > > > > Fix it there and don't try to use rdma-core headers to get kernel
> > ABI.
> > > > > > > >
> > > > > > > > Jason
> > > > > > >
> > > > > > > I wish you'd commented on this just a little sooner. I just resent
> > v3
> > > > > > > of this series... with rdma_cma.h included. :)
> > > > > > >
> > > > > > > How about the restrack/nldev code just translates the port space
> > from
> > > > > > > enum rdma_port_space to a new ABI enum, say
> > nldev_rdma_port_space, that
> > > > > > > i add to rdma_netlink.h? I'd hate to open the can of worms of
> > trying to
> > > > > > > split rdma_cma.h into uabi and no uabi headers. :(
> > > > > >
> > > > > > If port space is already part of the ABI there isn't much reason to
> > > > > > translate it.
> > > > > >
> > > > > > You just need to pick the right header to put it in, since it is a verbs
> > > > > > define it doesn't belong in the netlink header.
> > > > >
> > > > > I completely understand Steve's concerns.
> > > > >
> > > > > I tried to do such thing (expose kernel headers) in first incarnation of
> > > > > rdmatool with attempt to clean IB/core as well to ensure that we
> > won't expose
> > > > > anything that is not implemented. It didn't go well.
> > > >
> > > > rdma-core is now using the kernel uapi/ headers natively, seems to be
> > > > going OK. What problem did you face?
> > >
> > > I didn't agree to move to UAPI defines which are not implemented and
> > not
> > > used in the kernel, so I sent small number of patches similar to those [1,
> > 2].
> > >
> > > Those patches were rejected.
> > >
> > > So please don't mix Steve's need to use 3 defines with very large and
> > > painful task to expose proper UAPIs.
> >
> > Steve can just move the 3 defines he needs to the uapi, we are doing
> > this incrementally..
> >
> > rdma_core does not define kernel ABI and it is totally wrong to use
> > random constants from rdma_cma.h as kernel ABI.
> >
>
> Proposal:
>
> Since the cm_id port space is part of the rdma_ucm_create_id struct in include/uapi/rdma/rdma_user_cm.h, I'll move the rdma_port_space enum there. And then my iproute2 series will have to add a copy of rdma_user_cm.h locally into rdma/include/uapi/rdma, right?
>
> Will that work for everyone?
You need to remove _PS from that structure and from the kernel with
justification that it is safe to do.
Thanks
>
> Steve.
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v3 iproute2 1/1] tc: fix conversion types when printing actions unsigned values
From: Stephen Hemminger @ 2018-03-27 16:02 UTC (permalink / raw)
To: Roman Mashak; +Cc: netdev, jhs, xiyou.wangcong, jiri
In-Reply-To: <1521493541-31340-1-git-send-email-mrv@mojatatu.com>
On Mon, 19 Mar 2018 17:05:41 -0400
Roman Mashak <mrv@mojatatu.com> wrote:
> diff --git a/tc/m_gact.c b/tc/m_gact.c
> index 16c4413f4217..52022415db48 100644
> --- a/tc/m_gact.c
> +++ b/tc/m_gact.c
> @@ -194,7 +194,7 @@ print_gact(struct action_util *au, FILE *f, struct rtattr *arg)
> print_string(PRINT_ANY, "random_type", "\n\t random type %s",
> prob_n2a(pp->ptype));
> print_action_control(f, " ", pp->paction, " ");
> - print_int(PRINT_ANY, "val", "val %d", pp->pval);
> + print_int(PRINT_ANY, "val", "val %u", pp->pval);
> close_json_object();
This needs to be print_uint in order to work correctly with json output.
Also, please consider doing json on all the match types in a later patch
for net-next.
^ 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