* RE: [PATCH v2 1/5] netlink: remove NLA_NESTED_COMPAT
From: David Laight @ 2018-09-20 14:54 UTC (permalink / raw)
To: 'Johannes Berg', linux-wireless@vger.kernel.org,
netdev@vger.kernel.org
Cc: David Ahern, Johannes Berg
In-Reply-To: <20180919194905.16462-2-johannes@sipsolutions.net>
From: Johannes Berg
> Sent: 19 September 2018 20:49
> This isn't used anywhere, so we might as well get rid of it.
>
> Reviewed-by: David Ahern <dsahern@gmail.com>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
> include/net/netlink.h | 2 --
> lib/nlattr.c | 11 -----------
> 2 files changed, 13 deletions(-)
>
> diff --git a/include/net/netlink.h b/include/net/netlink.h
> index 318b1ded3833..b680fe365e91 100644
> --- a/include/net/netlink.h
> +++ b/include/net/netlink.h
> @@ -172,7 +172,6 @@ enum {
> NLA_FLAG,
> NLA_MSECS,
> NLA_NESTED,
> - NLA_NESTED_COMPAT,
> NLA_NUL_STRING,
> NLA_BINARY,
> NLA_S8,
...
Is it safe to remove an item from this emun ?
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH RFT net-next 0/2] net: phy: Eliminate unnecessary soft
From: Andrew Lunn @ 2018-09-20 14:58 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, David S. Miller, nbd, cphealy, harini.katakam, afleming,
agust, arnd, asmirnov, avi.kp.137, avorontsov, baijiaju1990, benh,
charles-antoine.couret, clemens.gruber, colin.king, cyril,
david.thomson, ddaney, dongsheng.wang, dwmw2, eha, houjingj, jeff,
Jingju.Hou, Jisheng.Zhang, johan, Kapil.Juneja, kim.phillips,
linyunsheng, madalin.bucur, michael, mic
In-Reply-To: <20180919013505.11347-1-f.fainelli@gmail.com>
On Tue, Sep 18, 2018 at 06:35:03PM -0700, Florian Fainelli wrote:
> Hi all,
>
> This patch series eliminates unnecessary software resets of the PHY.
> This should hopefully not break anybody's hardware; but I would
> appreciate testing to make sure this is is the case.
>
> Sorry for this long email list, I wanted to make sure I reached out to
> all people who made changes to the Marvell PHY driver.
Hi Florian
I want to test this, but i'm battling other issues at the moment and
not got around to it.
But i see it has been tested by two people. So i think we should merge
it, and see if anybody screams.
Andrew
^ permalink raw reply
* [PATCH] net: neterion: vxge: Remove unnecessary parentheses
From: Nathan Chancellor @ 2018-09-20 20:37 UTC (permalink / raw)
To: Jon Mason, David S. Miller; +Cc: netdev, linux-kernel, Nathan Chancellor
Clang warns when multiple pairs of parentheses are used for a single
conditional statement.
drivers/net/ethernet/neterion/vxge/vxge-traffic.c:2265:31: warning:
equality comparison with extraneous parentheses [-Wparentheses-equality]
if ((hldev->config.intr_mode ==
VXGE_HW_INTR_MODE_MSIX_ONE_SHOT))
~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/neterion/vxge/vxge-traffic.c:2265:31: note: remove
extraneous parentheses around the comparison to silence this warning
if ((hldev->config.intr_mode ==
VXGE_HW_INTR_MODE_MSIX_ONE_SHOT))
~ ^ ~
drivers/net/ethernet/neterion/vxge/vxge-traffic.c:2265:31: note: use '='
to turn this equality comparison into an assignment
if ((hldev->config.intr_mode ==
VXGE_HW_INTR_MODE_MSIX_ONE_SHOT))
^~
=
1 warning generated.
Link: https://github.com/ClangBuiltLinux/linux/issues/124
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
drivers/net/ethernet/neterion/vxge/vxge-traffic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/neterion/vxge/vxge-traffic.c b/drivers/net/ethernet/neterion/vxge/vxge-traffic.c
index 30e5cdc52eb6..f7a0d1d5885e 100644
--- a/drivers/net/ethernet/neterion/vxge/vxge-traffic.c
+++ b/drivers/net/ethernet/neterion/vxge/vxge-traffic.c
@@ -2262,7 +2262,7 @@ void vxge_hw_vpath_msix_clear(struct __vxge_hw_vpath_handle *vp, int msix_id)
{
struct __vxge_hw_device *hldev = vp->vpath->hldev;
- if ((hldev->config.intr_mode == VXGE_HW_INTR_MODE_MSIX_ONE_SHOT))
+ if (hldev->config.intr_mode == VXGE_HW_INTR_MODE_MSIX_ONE_SHOT)
__vxge_hw_pio_mem_write32_upper(
(u32) vxge_bVALn(vxge_mBIT((msix_id >> 2)), 0, 32),
&hldev->common_reg->clr_msix_one_shot_vec[msix_id % 4]);
--
2.19.0
^ permalink raw reply related
* Re: [PATCH net] ixgbe: check return value of napi_complete_done()
From: Eric Dumazet @ 2018-09-20 20:35 UTC (permalink / raw)
To: Song Liu, netdev; +Cc: intel-wired-lan, kernel-team, stable, Jeff Kirsher
In-Reply-To: <20180920190113.490005-1-songliubraving@fb.com>
On 09/20/2018 12:01 PM, Song Liu wrote:
> The NIC driver should only enable interrupts when napi_complete_done()
> returns true. This patch adds the check for ixgbe.
>
> Cc: stable@vger.kernel.org # 4.10+
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Suggested-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Song Liu <songliubraving@fb.com>
> ---
Well, unfortunately we do not know why this is needed,
this is why I have not yet sent this patch formally.
netpoll has correct synchronization :
poll_napi() places into napi->poll_owner current cpu number before calling poll_one_napi()
netpoll_poll_lock() does also use napi->poll_owner
When netpoll calls ixgbe poll() method, it passed a budget of 0,
meaning napi_complete_done() is not called.
As long as we can not explain the problem properly in the changelog,
we should investigate, otherwise we will probably see coming dozens of patches
trying to fix a 'potential hazard'.
Thanks.
^ permalink raw reply
* Re: [PATCH] smc: generic netlink family should be __ro_after_init
From: David Miller @ 2018-09-20 14:50 UTC (permalink / raw)
To: johannes; +Cc: linux-s390, netdev, ubraun, johannes.berg
In-Reply-To: <20180920072730.17584-1-johannes@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Thu, 20 Sep 2018 09:27:30 +0200
> From: Johannes Berg <johannes.berg@intel.com>
>
> The generic netlink family is only initialized during module init,
> so it should be __ro_after_init like all other generic netlink
> families.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Looks good, applied.
^ permalink raw reply
* Re: [PATCH net] mlxsw: spectrum: Bump required firmware version
From: David Miller @ 2018-09-20 14:49 UTC (permalink / raw)
To: idosch; +Cc: netdev, jiri, petrm, mlxsw
In-Reply-To: <20180920063145.12770-1-idosch@mellanox.com>
From: Ido Schimmel <idosch@mellanox.com>
Date: Thu, 20 Sep 2018 09:31:45 +0300
> From: Petr Machata <petrm@mellanox.com>
>
> MC-aware mode was introduced to mlxsw in commit 7b8195306694 ("mlxsw: spectrum:
> Configure MC-aware mode on mlxsw ports") and fixed up later in commit
> 3a3539cd3632 ("mlxsw: spectrum_buffers: Set up a dedicated pool for BUM
> traffic"). As the final piece of puzzle, a firmware issue whereby a wrong
> priority was assigned to BUM traffic was corrected in FW version 13.1703.4.
> Therefore require this FW version in the driver.
>
> Fixes: 7b8195306694 ("mlxsw: spectrum: Configure MC-aware mode on mlxsw ports")
> Signed-off-by: Petr Machata <petrm@mellanox.com>
> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 00/13] mlxsw: Further MC-awareness configuration
From: David Miller @ 2018-09-20 14:47 UTC (permalink / raw)
To: idosch; +Cc: netdev, jiri, petrm, mlxsw
In-Reply-To: <20180920062136.11888-1-idosch@mellanox.com>
From: Ido Schimmel <idosch@mellanox.com>
Date: Thu, 20 Sep 2018 09:21:23 +0300
> Petr says:
>
> Due to an issue in Spectrum chips, when unicast traffic shares the same
> queue as BUM traffic, and there is congestion, the BUM traffic is
> admitted to the queue anyway, thus pushing out all UC traffic. In order
> to give unicast traffic precedence over BUM traffic, multicast-aware
> mode is now configured on all ports. Under MC-aware mode, egress TCs
> 8..15 are used for BUM traffic, which has its own dedicated pool.
>
> This patch set improves the way that the MC pool and the higher-order
> TCs are integrated into the system.
Series applied, thanks.
> Then in patch #13 the selftest itself is added.
Just wanted to say I'm really happy with the selftests that exist
for all of the problems that have been fixed recently in mlxsw.
^ permalink raw reply
* Re: [PATCH bpf-next 2/2] xsk: fix bug when trying to use both copy and zero-copy on one queue id
From: Jakub Kicinski @ 2018-09-20 14:41 UTC (permalink / raw)
To: Magnus Karlsson
Cc: ys114321, Karlsson, Magnus, Björn Töpel, ast,
Daniel Borkmann, Network Development
In-Reply-To: <CAJ8uoz2m2amnk_aFwpyGg8Ss20vbt5EeVbXLEXQeO+gfEBc_cg@mail.gmail.com>
On Thu, 20 Sep 2018 11:17:17 +0200, Magnus Karlsson wrote:
> On Wed, Sep 19, 2018 at 9:18 AM Magnus Karlsson wrote:
> > On Wed, Sep 19, 2018 at 3:58 AM Jakub Kicinski wrote:
> > >
> > > On Tue, 18 Sep 2018 10:22:11 -0700, Y Song wrote:
> > > > > +/* The umem is stored both in the _rx struct and the _tx struct as we do
> > > > > + * not know if the device has more tx queues than rx, or the opposite.
> > > > > + * This might also change during run time.
> > > > > + */
> > > > > +static void xdp_reg_umem_at_qid(struct net_device *dev, struct xdp_umem *umem,
> > > > > + u16 queue_id)
> > > > > +{
> > > > > + if (queue_id < dev->real_num_rx_queues)
> > > > > + dev->_rx[queue_id].umem = umem;
> > > > > + if (queue_id < dev->real_num_tx_queues)
> > > > > + dev->_tx[queue_id].umem = umem;
> > > > > +}
> > > > > +
> > > > > +static struct xdp_umem *xdp_get_umem_from_qid(struct net_device *dev,
> > > > > + u16 queue_id)
> > > > > +{
> > > > > + if (queue_id < dev->real_num_rx_queues)
> > > > > + return dev->_rx[queue_id].umem;
> > > > > + if (queue_id < dev->real_num_tx_queues)
> > > > > + return dev->_tx[queue_id].umem;
> > > > > +
> > > > > + return NULL;
> > > > > +}
> > > > > +
> > > > > +static void xdp_clear_umem_at_qid(struct net_device *dev, u16 queue_id)
> > > > > +{
> > > > > + /* Zero out the entry independent on how many queues are configured
> > > > > + * at this point in time, as it might be used in the future.
> > > > > + */
> > > > > + if (queue_id < dev->num_rx_queues)
> > > > > + dev->_rx[queue_id].umem = NULL;
> > > > > + if (queue_id < dev->num_tx_queues)
> > > > > + dev->_tx[queue_id].umem = NULL;
> > > > > +}
> > > > > +
> > > >
> > > > I am sure whether the following scenario can happen or not.
> > > > Could you clarify?
> > > > 1. suppose initially we have num_rx_queues = num_tx_queues = 10
> > > > xdp_reg_umem_at_qid() set umem1 to queue_id = 8
> > > > 2. num_tx_queues is changed to 5
> > > > 3. xdp_clear_umem_at_qid() is called for queue_id = 8,
> > > > and dev->_rx[8].umum = 0.
> > > > 4. xdp_reg_umem_at_qid() is called gain to set for queue_id = 8
> > > > dev->_rx[8].umem = umem2
> > > > 5. num_tx_queues is changed to 10
> > > > Now dev->_rx[8].umem != dev->_tx[8].umem, is this possible and is it
> > > > a problem?
> > >
> > > Plus IIRC the check of qid vs real_num_[rt]x_queues in xsk_bind() is
> > > not under rtnl_lock so it doesn't count for much. Why not do all the
> > > checks against num_[rt]x_queues here, instead of real_..?
> >
> > You are correct, two separate rtnl_lock regions is broken. Will spin a
> > v2 tomorrow when I am back in the office.
> >
> > Thanks Jakub for catching this. I really appreciate you reviewing my code.
>
> Sorry, forgot to answer your question about why real_num_ instead of
> num_. If we used num_ instead, we would get a behavior where we can
> open a socket on a queue id, let us say 10, that will not be active if
> real_num_ is below 10. So no traffic will flow on this socket until we
> issue an ethtool command to state that we have 10 or more queues
> configured. While this behavior is sane (and consistent), I believe it
> will lead to a more complicated driver implementation, break the
> current uapi (since we will return an error if you try to bind to a
> queue id that is not active at the moment) and I like my suggestion
> below better :-).
>
> What I would like to suggest is that the current model at bind() is
> kept, that is it will fail if you try to bind to a non-active queue
> id. But we add some code in ethool_set_channels in net/core/ethtool.c
> to check if you have an active af_xdp socket on a queue id and try to
> make it inactive, we return an error and disallow the change. This
> would IMHO be like not allowing a load module to be unloaded if
> someone is using it or not allowing unmounting a file system if files
> are in use. What do you think? If you think this is the right way to
> go, I can implement this in a follow up patch or in this patch set if
> that makes more sense, let me know. This suggestion would actually
> make it simpler to implement zero-copy support in the driver. Some of
> the complications we are having today is due to the fact that ethtool
> can come in from the side and change things for us when an AF_XDP
> socket is active on a queue id. And implementing zero copy support in
> the driver needs to get simpler IMO.
>
> Please let me know what you think.
I think I'd like that:
https://patchwork.ozlabs.org/project/netdev/list/?series=57843&state=*
in particular:
https://patchwork.ozlabs.org/patch/949891/
;)
If we fix the resize indeed you could stick to real_* but to be honest,
I'm not entirely clear on what the advantage would be? The arrays are
sized to num..queues, do you envision a use case where having
xdp_get_umem_from_qid() return a NULL when asked for qid > real_..
would help? Is there one for xdp_reg_umem_at_qid()
I have no strong preference here, just wondering.
^ permalink raw reply
* Re: [PATCH rdma-next 1/5] RDMA/core: Provide getter and setter to access IB device name
From: Steve Wise @ 2018-09-20 14:32 UTC (permalink / raw)
To: Leon Romanovsky, Doug Ledford, Jason Gunthorpe
Cc: Leon Romanovsky, RDMA mailing list, linux-s390, Ursula Braun,
David S. Miller, netdev, Selvin Xavier, Steve Wise, Lijun Ou,
Shiraz Saleem, Ariel Elior, Christian Benvenuti, Adit Ranadive,
Dennis Dalessandro
In-Reply-To: <20180920112202.9181-2-leon@kernel.org>
On 9/20/2018 6:21 AM, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Prepare IB device name field to rename operation by ensuring that all
> accesses to it are protected with lock and users don't see part of name.
>
> The protection is done with global device_lock because it is used in
> allocation and deallocation phases. At this stage, this lock is not
> busy and easily can be moved to be per-device, once it will be needed.
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
> drivers/infiniband/core/device.c | 24 +++++++++++++++++++++++-
> include/rdma/ib_verbs.h | 8 +++++++-
> 2 files changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> index 5a680a88aa87..3270cde6d806 100644
> --- a/drivers/infiniband/core/device.c
> +++ b/drivers/infiniband/core/device.c
> @@ -170,6 +170,14 @@ static struct ib_device *__ib_device_get_by_name(const char *name)
> return NULL;
> }
>
> +void ib_device_get_name(struct ib_device *ibdev, char *name)
> +{
> + down_read(&lists_rwsem);
> + strlcpy(name, ibdev->name, IB_DEVICE_NAME_MAX);
> + up_read(&lists_rwsem);
> +}
> +EXPORT_SYMBOL(ib_device_get_name);
> +
> static int alloc_name(char *name)
> {
> unsigned long *inuse;
> @@ -202,6 +210,21 @@ static int alloc_name(char *name)
> return 0;
> }
>
> +int ib_device_alloc_name(struct ib_device *ibdev, const char *pattern)
> +{
> + int ret = 0;
> +
> + mutex_lock(&device_mutex);
> + strlcpy(ibdev->name, pattern, IB_DEVICE_NAME_MAX);
> + if (strchr(ibdev->name, '%'))
> + ret = alloc_name(ibdev->name);
> +
> + mutex_unlock(&device_mutex);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(ib_device_alloc_name);
> +
> static void ib_device_release(struct device *device)
> {
> struct ib_device *dev = container_of(device, struct ib_device, dev);
> @@ -499,7 +522,6 @@ int ib_register_device(struct ib_device *device,
> ret = alloc_name(device->name);
> if (ret)
> goto out;
> - }
I don't think this is correct...
>
> if (ib_device_check_mandatory(device)) {
> ret = -EINVAL;
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index e764ed1f6025..66660e7b9854 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -2260,6 +2260,11 @@ struct ib_device {
> /* Do not access @dma_device directly from ULP nor from HW drivers. */
> struct device *dma_device;
>
> + /*
> + * Do not access @name directly,
> + * use ib_device_get_name()/ib_device_alloc_name()
> + * and don't assume that it can't change after access.
> + */
> char name[IB_DEVICE_NAME_MAX];
>
> struct list_head event_handler_list;
> @@ -2638,7 +2643,8 @@ struct ib_device *ib_alloc_device(size_t size);
> void ib_dealloc_device(struct ib_device *device);
>
> void ib_get_device_fw_str(struct ib_device *device, char *str);
> -
> +int ib_device_alloc_name(struct ib_device *ibdev, const char *pattern);
> +void ib_device_get_name(struct ib_device *ibdev, char *name);
> int ib_register_device(struct ib_device *device,
> int (*port_callback)(struct ib_device *,
> u8, struct kobject *));
> --
> 2.14.4
>
^ permalink raw reply
* Re: WARNING: refcount bug in igmp_start_timer
From: syzbot @ 2018-09-20 20:06 UTC (permalink / raw)
To: davem, kuznet, linux-kernel, netdev, syzkaller-bugs, yoshfuji
In-Reply-To: <0000000000002b42040573b8495a@google.com>
syzbot has found a reproducer for the following crash on:
HEAD commit: ae596de1a0c8 Compiler Attributes: naked can be shared
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=15bdb156400000
kernel config: https://syzkaller.appspot.com/x/.config?x=5fa12be50bca08d8
dashboard link: https://syzkaller.appspot.com/bug?extid=e28037ac1c96d2a86e89
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=178537da400000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+e28037ac1c96d2a86e89@syzkaller.appspotmail.com
IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
8021q: adding VLAN 0 to HW filter on device team0
------------[ cut here ]------------
refcount_t: increment on 0; use-after-free.
WARNING: CPU: 1 PID: 13494 at lib/refcount.c:153
refcount_inc_checked+0x5d/0x70 lib/refcount.c:153
Kernel panic - not syncing: panic_on_warn set ...
CPU: 1 PID: 13494 Comm: syz-executor0 Not tainted 4.19.0-rc4+ #26
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c4/0x2b4 lib/dump_stack.c:113
panic+0x238/0x4e7 kernel/panic.c:184
__warn.cold.8+0x163/0x1ba kernel/panic.c:536
report_bug+0x254/0x2d0 lib/bug.c:186
fixup_bug arch/x86/kernel/traps.c:178 [inline]
do_error_trap+0x1fc/0x4d0 arch/x86/kernel/traps.c:296
do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:316
invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:993
RIP: 0010:refcount_inc_checked+0x5d/0x70 lib/refcount.c:153
Code: 1d a2 83 91 06 31 ff 89 de e8 7f 8e ef fd 84 db 75 df e8 a6 8d ef fd
48 c7 c7 40 82 44 88 c6 05 82 83 91 06 01 e8 63 74 b9 fd <0f> 0b eb c3 0f
1f 44 00 00 66 2e 0f 1f 84 00 00 00 00 00 55 48 89
RSP: 0018:ffff8801c7fdeca0 EFLAGS: 00010282
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff8164fce5 RDI: 0000000000000005
RBP: ffff8801c7fdeca8 R08: ffff8801c67da1c0 R09: ffffed003b5e3ee2
R10: ffffed003b5e3ee2 R11: ffff8801daf1f717 R12: 0000000000000000
R13: 0000000000000008 R14: ffff8801b465f640 R15: dffffc0000000000
igmp_start_timer+0xaf/0xe0 net/ipv4/igmp.c:217
igmp_mod_timer net/ipv4/igmp.c:255 [inline]
igmp_heard_query net/ipv4/igmp.c:1026 [inline]
igmp_rcv+0x190e/0x3020 net/ipv4/igmp.c:1061
ip_local_deliver_finish+0x2e9/0xda0 net/ipv4/ip_input.c:215
NF_HOOK include/linux/netfilter.h:287 [inline]
ip_local_deliver+0x1e9/0x750 net/ipv4/ip_input.c:256
dst_input include/net/dst.h:450 [inline]
ip_rcv_finish+0x1f9/0x300 net/ipv4/ip_input.c:415
NF_HOOK include/linux/netfilter.h:287 [inline]
ip_rcv+0xed/0x610 net/ipv4/ip_input.c:524
__netif_receive_skb_one_core+0x14d/0x200 net/core/dev.c:4891
__netif_receive_skb+0x2c/0x1e0 net/core/dev.c:5001
netif_receive_skb_internal+0x12c/0x620 net/core/dev.c:5104
napi_frags_finish net/core/dev.c:5642 [inline]
napi_gro_frags+0x75a/0xc90 net/core/dev.c:5715
tun_get_user+0x31d5/0x42a0 drivers/net/tun.c:1965
tun_chr_write_iter+0xb9/0x154 drivers/net/tun.c:2010
call_write_iter include/linux/fs.h:1808 [inline]
do_iter_readv_writev+0x8b0/0xa80 fs/read_write.c:680
do_iter_write+0x185/0x5f0 fs/read_write.c:959
vfs_writev+0x1f1/0x360 fs/read_write.c:1004
do_writev+0x11a/0x310 fs/read_write.c:1039
__do_sys_writev fs/read_write.c:1112 [inline]
__se_sys_writev fs/read_write.c:1109 [inline]
__x64_sys_writev+0x75/0xb0 fs/read_write.c:1109
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x457531
Code: 75 14 b8 14 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 54 b5 fb ff c3 48
83 ec 08 e8 1a 2d 00 00 48 89 04 24 b8 14 00 00 00 0f 05 <48> 8b 3c 24 48
89 c2 e8 63 2d 00 00 48 89 d0 48 83 c4 08 48 3d 01
RSP: 002b:00007f1448a89ba0 EFLAGS: 00000293 ORIG_RAX: 0000000000000014
RAX: ffffffffffffffda RBX: 000000000000002a RCX: 0000000000457531
RDX: 0000000000000001 RSI: 00007f1448a89bf0 RDI: 00000000000000f0
RBP: 0000000020000240 R08: 00000000000000f0 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000293 R12: 00000000ffffffff
R13: 00000000004d7938 R14: 00000000004c48b4 R15: 0000000000000000
Kernel Offset: disabled
Rebooting in 86400 seconds..
^ permalink raw reply
* Re: [PATCH v2 0/2] hv_netvsc: associate VF and PV device by serial number
From: Lorenzo Pieralisi @ 2018-09-20 14:18 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: kys, haiyangz, sthemmin, devel, netdev, linux-pci
In-Reply-To: <20180914195457.20433-1-sthemmin@microsoft.com>
On Fri, Sep 14, 2018 at 12:54:55PM -0700, Stephen Hemminger wrote:
> The Hyper-V implementation of PCI controller has concept of 32 bit serial number
> (not to be confused with PCI-E serial number). This value is sent in the protocol
> from the host to indicate SR-IOV VF device is attached to a synthetic NIC.
>
> Using the serial number (instead of MAC address) to associate the two devices
> avoids lots of potential problems when there are duplicate MAC addresses from
> tunnels or layered devices.
>
> The patch set is broken into two parts, one is for the PCI controller
> and the other is for the netvsc device. Normally, these go through different
> trees but sending them together here for better review. The PCI changes
> were submitted previously, but the main review comment was "why do you
> need this?". This is why.
The question was more whether we should convert this serial number into
a PCI slot number (that has user space visibility and that is what you are
after) to improve the current matching, I do not question why you need
it, just for the records.
Lorenzo
> v2 - slot name can be shorter.
> remove locking when creating pci_slots; see comment for explaination
>
> Stephen Hemminger (2):
> PCI: hv: support reporting serial number as slot information
> hv_netvsc: pair VF based on serial number
>
> drivers/net/hyperv/netvsc.c | 3 ++
> drivers/net/hyperv/netvsc_drv.c | 58 ++++++++++++++++-------------
> drivers/pci/controller/pci-hyperv.c | 37 ++++++++++++++++++
> 3 files changed, 73 insertions(+), 25 deletions(-)
>
> --
> 2.18.0
>
^ permalink raw reply
* Re: [PATCH net] sctp: update dst pmtu with the correct daddr
From: Marcelo Ricardo Leitner @ 2018-09-20 14:17 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, davem, Neil Horman
In-Reply-To: <dbed84b3964b13a0c15b3a5c8e825940819a2918.1537435648.git.lucien.xin@gmail.com>
On Thu, Sep 20, 2018 at 05:27:28PM +0800, Xin Long wrote:
> When processing pmtu update from an icmp packet, it calls .update_pmtu
> with sk instead of skb in sctp_transport_update_pmtu.
>
> However for sctp, the daddr in the transport might be different from
> inet_sock->inet_daddr or sk->sk_v6_daddr, which is used to update or
> create the route cache. The incorrect daddr will cause a different
> route cache created for the path.
>
> So before calling .update_pmtu, inet_sock->inet_daddr/sk->sk_v6_daddr
> should be updated with the daddr in the transport, and update it back
> after it's done.
>
> The issue has existed since route exceptions introduction.
>
> Fixes: 4895c771c7f0 ("ipv4: Add FIB nexthop exceptions.")
> Reported-by: ian.periam@dialogic.com
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
> net/sctp/transport.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/net/sctp/transport.c b/net/sctp/transport.c
> index 12cac85..033696e 100644
> --- a/net/sctp/transport.c
> +++ b/net/sctp/transport.c
> @@ -260,6 +260,7 @@ void sctp_transport_pmtu(struct sctp_transport *transport, struct sock *sk)
> bool sctp_transport_update_pmtu(struct sctp_transport *t, u32 pmtu)
> {
> struct dst_entry *dst = sctp_transport_dst_check(t);
> + struct sock *sk = t->asoc->base.sk;
> bool change = true;
>
> if (unlikely(pmtu < SCTP_DEFAULT_MINSEGMENT)) {
> @@ -271,12 +272,19 @@ bool sctp_transport_update_pmtu(struct sctp_transport *t, u32 pmtu)
> pmtu = SCTP_TRUNC4(pmtu);
>
> if (dst) {
> - dst->ops->update_pmtu(dst, t->asoc->base.sk, NULL, pmtu);
> + struct sctp_pf *pf = sctp_get_pf_specific(dst->ops->family);
> + union sctp_addr addr;
> +
> + pf->af->from_sk(&addr, sk);
> + pf->to_sk_daddr(&t->ipaddr, sk);
> + dst->ops->update_pmtu(dst, sk, NULL, pmtu);
> + pf->to_sk_daddr(&addr, sk);
> +
> dst = sctp_transport_dst_check(t);
> }
>
> if (!dst) {
> - t->af_specific->get_dst(t, &t->saddr, &t->fl, t->asoc->base.sk);
> + t->af_specific->get_dst(t, &t->saddr, &t->fl, sk);
> dst = t->dst;
> }
>
> --
> 2.1.0
>
^ permalink raw reply
* Re: [PATCH 0/2] net/sched: Add hardware specific counters to TC actions
From: Jakub Kicinski @ 2018-09-20 14:14 UTC (permalink / raw)
To: Eelco Chaudron
Cc: David Miller, netdev, jhs, xiyou.wangcong, jiri, simon.horman,
Marcelo Ricardo Leitner, louis.peens, Paolo, Davide Caratti
In-Reply-To: <C997079A-1449-4D38-84AC-FF6E57F66ED3@redhat.com>
On Thu, 20 Sep 2018 09:14:08 +0200, Eelco Chaudron wrote:
> Is there anything else blocking from getting this into net-next?
>
> I still think this patch is beneficial for the full user experience, and
> I’ve got requests from QA and others for this.
Not from my perspective, the numbers look okay, feel free to repost!
^ permalink raw reply
* Re: array bounds warning in xfrm_output_resume
From: Florian Westphal @ 2018-09-20 14:06 UTC (permalink / raw)
To: David Ahern; +Cc: Florian Westphal, netdev@vger.kernel.org
In-Reply-To: <8808f8a3-ea1c-f91b-8288-cc4952086f04@gmail.com>
David Ahern <dsahern@gmail.com> wrote:
> > $ make O=kbuild/perf -j 24 -s
> > In file included from /home/dsa/kernel-3.git/include/linux/kernel.h:10:0,
> > from /home/dsa/kernel-3.git/include/linux/list.h:9,
> > from /home/dsa/kernel-3.git/include/linux/module.h:9,
> > from /home/dsa/kernel-3.git/net/xfrm/xfrm_output.c:13:
> > /home/dsa/kernel-3.git/net/xfrm/xfrm_output.c: In function
> > ‘xfrm_output_resume’:
> > /home/dsa/kernel-3.git/include/linux/compiler.h:252:20: warning: array
> > subscript is above array bounds [-Warray-bounds]
> > __read_once_size(&(x), __u.__c, sizeof(x)); \
Does this thing avoid the warning?
diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -143,6 +143,8 @@ int xfrm_output_resume(struct sk_buff *skb, int err)
struct net *net = xs_net(skb_dst(skb)->xfrm);
while (likely((err = xfrm_output_one(skb, err)) == 0)) {
+ unsigned short pf;
+
nf_reset(skb);
err = skb_dst(skb)->ops->local_out(net, skb->sk, skb);
@@ -152,11 +154,19 @@ int xfrm_output_resume(struct sk_buff *skb, int err)
if (!skb_dst(skb)->xfrm)
return dst_output(net, skb->sk, skb);
- err = nf_hook(skb_dst(skb)->ops->family,
- NF_INET_POST_ROUTING, net, skb->sk, skb,
- NULL, skb_dst(skb)->dev, xfrm_output2);
- if (unlikely(err != 1))
- goto out;
+ pf = skb_dst(skb)->ops->family;
+ switch (pf) {
+ case AF_INET: /* fallthrough */
+ case AF_INET6:
+ err = nf_hook(pf,
+ NF_INET_POST_ROUTING, net, skb->sk, skb,
+ NULL, skb_dst(skb)->dev, xfrm_output2);
+ if (unlikely(err != 1))
+ goto out;
+ break;
+ default:
+ break;
+ }
}
if (err == -EINPROGRESS)
^ permalink raw reply
* Re: [PATCH perf 3/3] tools/perf: recognize and process RECORD_MMAP events for bpf progs
From: Arnaldo Carvalho de Melo @ 2018-09-20 14:05 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: David S . Miller, daniel, peterz, netdev, kernel-team
In-Reply-To: <20180920133617.GB19861@kernel.org>
Em Thu, Sep 20, 2018 at 10:36:17AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Sep 19, 2018 at 03:39:35PM -0700, Alexei Starovoitov escreveu:
> > Recognize JITed bpf prog load/unload events.
> > Add/remove kernel symbols accordingly.
> >
> > Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> > ---
> > tools/perf/util/machine.c | 27 +++++++++++++++++++++++++++
> > tools/perf/util/symbol.c | 13 +++++++++++++
> > tools/perf/util/symbol.h | 1 +
> > 3 files changed, 41 insertions(+)
> >
> > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> > index c4acd2001db0..ae4f8a0fdc7e 100644
> > --- a/tools/perf/util/machine.c
> > +++ b/tools/perf/util/machine.c
> > @@ -25,6 +25,7 @@
> > #include "sane_ctype.h"
> > #include <symbol/kallsyms.h>
> > #include <linux/mman.h>
> > +#include <linux/magic.h>
> >
> > static void __machine__remove_thread(struct machine *machine, struct thread *th, bool lock);
> >
> > @@ -1460,6 +1461,32 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
> > enum dso_kernel_type kernel_type;
> > bool is_kernel_mmap;
> >
> > + /* process JITed bpf programs load/unload events */
> > + if (event->mmap.pid == ~0u && event->mmap.tid == BPF_FS_MAGIC) {
>
>
> So, this would be in machine__process_kernel_munmap-event(machine), etc,
> no check for BPF_FS_MAGIC would be needed with a PERF_RECORD_MUNMAP.
>
> > + struct symbol *sym;
> > + u64 ip;
> > +
> > + map = map_groups__find(&machine->kmaps, event->mmap.start);
> > + if (!map) {
> > + pr_err("No kernel map for IP %lx\n", event->mmap.start);
> > + goto out_problem;
> > + }
> > + ip = event->mmap.start - map->start + map->pgoff;
> > + if (event->mmap.filename[0]) {
> > + sym = symbol__new(ip, event->mmap.len, 0, 0,
> > + event->mmap.filename);
>
> Humm, so the bpf program would be just one symbol... bpf-to-bpf calls
> will be to a different bpf program, right?
>
> /me goes to read https://lwn.net/Articles/741773/
> "[PATCH bpf-next 00/13] bpf: introduce function calls"
After reading it, yeah, I think we need some way to access a symbol
table for a BPF program, and also its binary so that we can do
annotation, static (perf annotate) and live (perf top), was this already
considered? I think one can get the binary for a program giving
sufficient perms somehow, right? One other thing I need to catch up 8-)
- Arnaldo
> > + dso__insert_symbol(map->dso, sym);
> > + } else {
> > + if (symbols__erase(&map->dso->symbols, ip)) {
> > + pr_err("No bpf prog at IP %lx/%lx\n",
> > + event->mmap.start, ip);
> > + goto out_problem;
> > + }
> > + dso__reset_find_symbol_cache(map->dso);
> > + }
> > + return 0;
> > + }
> > +
> > /* If we have maps from kcore then we do not need or want any others */
> > if (machine__uses_kcore(machine))
> > return 0;
> > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> > index d188b7588152..0653f313661d 100644
> > --- a/tools/perf/util/symbol.c
> > +++ b/tools/perf/util/symbol.c
> > @@ -353,6 +353,19 @@ static struct symbol *symbols__find(struct rb_root *symbols, u64 ip)
> > return NULL;
> > }
> >
> > +int symbols__erase(struct rb_root *symbols, u64 ip)
> > +{
> > + struct symbol *s;
> > +
> > + s = symbols__find(symbols, ip);
> > + if (!s)
> > + return -ENOENT;
> > +
> > + rb_erase(&s->rb_node, symbols);
> > + symbol__delete(s);
> > + return 0;
> > +}
> > +
> > static struct symbol *symbols__first(struct rb_root *symbols)
> > {
> > struct rb_node *n = rb_first(symbols);
> > diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> > index f25fae4b5743..92ef31953d9a 100644
> > --- a/tools/perf/util/symbol.h
> > +++ b/tools/perf/util/symbol.h
> > @@ -310,6 +310,7 @@ char *dso__demangle_sym(struct dso *dso, int kmodule, const char *elf_name);
> >
> > void __symbols__insert(struct rb_root *symbols, struct symbol *sym, bool kernel);
> > void symbols__insert(struct rb_root *symbols, struct symbol *sym);
> > +int symbols__erase(struct rb_root *symbols, u64 ip);
> > void symbols__fixup_duplicate(struct rb_root *symbols);
> > void symbols__fixup_end(struct rb_root *symbols);
> > void map_groups__fixup_end(struct map_groups *mg);
> > --
> > 2.17.1
^ permalink raw reply
* Re: [PATCH bpf-next 2/3] bpf: emit RECORD_MMAP events for bpf prog load/unload
From: Peter Zijlstra @ 2018-09-20 13:56 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Alexei Starovoitov, David S . Miller, daniel, netdev, kernel-team
In-Reply-To: <20180920132545.GA19861@kernel.org>
On Thu, Sep 20, 2018 at 10:25:45AM -0300, Arnaldo Carvalho de Melo wrote:
> PeterZ provided a patch introducing PERF_RECORD_MUNMAP, went nowhere due
> to having to cope with munmapping parts of existing mmaps, etc.
>
> I'm still more in favour of introduce PERF_RECORD_MUNMAP, even if for
> now it would be used just in this clean case for undoing a
> PERF_RECORD_MMAP for a BPF program.
>
> The ABI is already complicated, starting to use something called
> PERF_RECORD_MMAP for unmmaping by just using a NULL name... too clever,
> I think.
Agreed, the PERF_RECORD_MUNMAP patch was fairly trivial, the difficult
part was getting the perf tool to dtrt for that use-case. But if we need
unmap events, doing the unmap record now is the right thing.
^ permalink raw reply
* Re: [PATCH][next-next][v2] netlink: avoid to allocate full skb when sending to many devices
From: Eric Dumazet @ 2018-09-20 13:51 UTC (permalink / raw)
To: Li RongQing, netdev
In-Reply-To: <a9b4a4b9-04c4-bf55-3f08-b7889524374a@gmail.com>
On 09/20/2018 06:43 AM, Eric Dumazet wrote:
>
> And lastly this patch looks way too complicated to me.
> You probably can write something much simpler.
Something like :
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 930d17fa906c9ebf1cf7b6031ce0a22f9f66c0e4..e0a81beb4f37751421dbbe794ccf3d5a46bdf900 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -278,22 +278,26 @@ static bool netlink_filter_tap(const struct sk_buff *skb)
return false;
}
-static int __netlink_deliver_tap_skb(struct sk_buff *skb,
+static int __netlink_deliver_tap_skb(struct sk_buff **pskb,
struct net_device *dev)
{
- struct sk_buff *nskb;
+ struct sk_buff *nskb, *skb = *pskb;
struct sock *sk = skb->sk;
int ret = -ENOMEM;
if (!net_eq(dev_net(dev), sock_net(sk)))
return 0;
- dev_hold(dev);
-
- if (is_vmalloc_addr(skb->head))
+ if (is_vmalloc_addr(skb->head)) {
nskb = netlink_to_full_skb(skb, GFP_ATOMIC);
- else
- nskb = skb_clone(skb, GFP_ATOMIC);
+ if (!nskb)
+ return -ENOMEM;
+ consume_skb(skb);
+ skb = nskb;
+ *pskb = skb;
+ }
+ dev_hold(dev);
+ nskb = skb_clone(skb, GFP_ATOMIC);
if (nskb) {
nskb->dev = dev;
nskb->protocol = htons((u16) sk->sk_protocol);
@@ -318,7 +322,7 @@ static void __netlink_deliver_tap(struct sk_buff *skb, struct netlink_tap_net *n
return;
list_for_each_entry_rcu(tmp, &nn->netlink_tap_all, list) {
- ret = __netlink_deliver_tap_skb(skb, tmp->dev);
+ ret = __netlink_deliver_tap_skb(&skb, tmp->dev);
if (unlikely(ret))
break;
}
^ permalink raw reply related
* [PATCH] DRIVERS: net: macsec: Fix multiple coding style issues
From: Romain Aviolat @ 2018-09-20 19:33 UTC (permalink / raw)
To: davem
Cc: elena.reshetova, ishkamiel, sd, dan.carpenter, felix.walter,
r.aviolat, netdev, linux-kernel
This patch fixes a couple of issues highlighted by checkpatch.pl:
* Missing a blank line after declarations
* Alignment should match open parenthesis
Signed-off-by: Romain Aviolat <r.aviolat@gmail.com>
---
drivers/net/macsec.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 7de88b33d5b9..4bb90b6867a2 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -64,9 +64,9 @@ struct macsec_eth_header {
#define MACSEC_NUM_AN 4 /* 2 bits for the association number */
-#define for_each_rxsc(secy, sc) \
+#define for_each_rxsc(secy, sc) \
for (sc = rcu_dereference_bh(secy->rx_sc); \
- sc; \
+ sc; \
sc = rcu_dereference_bh(sc->next))
#define for_each_rxsc_rtnl(secy, sc) \
for (sc = rtnl_dereference(secy->rx_sc); \
@@ -1142,6 +1142,7 @@ static rx_handler_result_t macsec_handle_frame(struct sk_buff **pskb)
list_for_each_entry_rcu(macsec, &rxd->secys, secys) {
struct macsec_rx_sc *sc = find_rx_sc(&macsec->secy, sci);
+
sc = sc ? macsec_rxsc_get(sc) : NULL;
if (sc) {
@@ -1584,7 +1585,6 @@ static struct macsec_rx_sa *get_rxsa_from_nl(struct net *net,
return rx_sa;
}
-
static const struct nla_policy macsec_genl_policy[NUM_MACSEC_ATTR] = {
[MACSEC_ATTR_IFINDEX] = { .type = NLA_U32 },
[MACSEC_ATTR_RXSC_CONFIG] = { .type = NLA_NESTED },
@@ -2156,7 +2156,7 @@ static int macsec_upd_rxsc(struct sk_buff *skb, struct genl_info *info)
}
static int copy_tx_sa_stats(struct sk_buff *skb,
- struct macsec_tx_sa_stats __percpu *pstats)
+ struct macsec_tx_sa_stats __percpu *pstats)
{
struct macsec_tx_sa_stats sum = {0, };
int cpu;
@@ -2176,7 +2176,7 @@ static int copy_tx_sa_stats(struct sk_buff *skb,
}
static int copy_rx_sa_stats(struct sk_buff *skb,
- struct macsec_rx_sa_stats __percpu *pstats)
+ struct macsec_rx_sa_stats __percpu *pstats)
{
struct macsec_rx_sa_stats sum = {0, };
int cpu;
@@ -2202,7 +2202,7 @@ static int copy_rx_sa_stats(struct sk_buff *skb,
}
static int copy_rx_sc_stats(struct sk_buff *skb,
- struct pcpu_rx_sc_stats __percpu *pstats)
+ struct pcpu_rx_sc_stats __percpu *pstats)
{
struct macsec_rx_sc_stats sum = {0, };
int cpu;
@@ -2266,7 +2266,7 @@ static int copy_rx_sc_stats(struct sk_buff *skb,
}
static int copy_tx_sc_stats(struct sk_buff *skb,
- struct pcpu_tx_sc_stats __percpu *pstats)
+ struct pcpu_tx_sc_stats __percpu *pstats)
{
struct macsec_tx_sc_stats sum = {0, };
int cpu;
@@ -2306,7 +2306,7 @@ static int copy_tx_sc_stats(struct sk_buff *skb,
}
static int copy_secy_stats(struct sk_buff *skb,
- struct pcpu_secy_stats __percpu *pstats)
+ struct pcpu_secy_stats __percpu *pstats)
{
struct macsec_dev_stats sum = {0, };
int cpu;
@@ -2962,13 +2962,11 @@ static int macsec_get_iflink(const struct net_device *dev)
return macsec_priv(dev)->real_dev->ifindex;
}
-
static int macsec_get_nest_level(struct net_device *dev)
{
return macsec_priv(dev)->nest_level;
}
-
static const struct net_device_ops macsec_netdev_ops = {
.ndo_init = macsec_dev_init,
.ndo_uninit = macsec_dev_uninit,
--
2.17.1
^ permalink raw reply related
* Re: [RFC PATCH ethtool] ethtool: better syntax for combinations of FEC modes
From: Michal Kubecek @ 2018-09-20 13:46 UTC (permalink / raw)
To: Edward Cree; +Cc: John W. Linville, netdev
In-Reply-To: <a5312743-9d54-b239-80ee-cc7aff77b6aa@solarflare.com>
On Wed, Sep 19, 2018 at 05:06:25PM +0100, Edward Cree wrote:
> Instead of commas, just have them as separate argvs.
>
> The parsing state machine might look heavyweight, but it makes it easy to add
> more parameters later and distinguish parameter names from encoding names.
>
> Suggested-by: Michal Kubecek <mkubecek@suse.cz>
> Signed-off-by: Edward Cree <ecree@solarflare.com>
> ---
Looks good, thank you.
Reviewed-by: Michal Kubecek <mkubecek@suse.cz>
> ethtool.8.in | 6 +++---
> ethtool.c | 63 ++++++++++++++++------------------------------------------
> test-cmdline.c | 10 +++++-----
> 3 files changed, 25 insertions(+), 54 deletions(-)
>
> diff --git a/ethtool.8.in b/ethtool.8.in
> index 414eaa1..7ea2cc0 100644
> --- a/ethtool.8.in
> +++ b/ethtool.8.in
> @@ -390,7 +390,7 @@ ethtool \- query or control network driver and hardware settings
> .B ethtool \-\-set\-fec
> .I devname
> .B encoding
> -.BR auto | off | rs | baser [ , ...]
> +.BR auto | off | rs | baser \ [...]
> .
> .\" Adjust lines (i.e. full justification) and hyphenate.
> .ad
> @@ -1120,11 +1120,11 @@ current FEC mode, the driver or firmware must take the link down
> administratively and report the problem in the system logs for users to correct.
> .RS 4
> .TP
> -.BR encoding\ auto | off | rs | baser [ , ...]
> +.BR encoding\ auto | off | rs | baser \ [...]
>
> Sets the FEC encoding for the device. Combinations of options are specified as
> e.g.
> -.B auto,rs
> +.B encoding auto rs
> ; the semantics of such combinations vary between drivers.
> .TS
> nokeep;
> diff --git a/ethtool.c b/ethtool.c
> index 9997930..2f7e96b 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -4979,39 +4979,6 @@ static int fecmode_str_to_type(const char *str)
> return 0;
> }
>
> -/* Takes a comma-separated list of FEC modes, returns the bitwise OR of their
> - * corresponding ETHTOOL_FEC_* constants.
> - * Accepts repetitions (e.g. 'auto,auto') and trailing comma (e.g. 'off,').
> - */
> -static int parse_fecmode(const char *str)
> -{
> - int fecmode = 0;
> - char buf[6];
> -
> - if (!str)
> - return 0;
> - while (*str) {
> - size_t next;
> - int mode;
> -
> - next = strcspn(str, ",");
> - if (next >= 6) /* Bad mode, longest name is 5 chars */
> - return 0;
> - /* Copy into temp buffer and nul-terminate */
> - memcpy(buf, str, next);
> - buf[next] = 0;
> - mode = fecmode_str_to_type(buf);
> - if (!mode) /* Bad mode encountered */
> - return 0;
> - fecmode |= mode;
> - str += next;
> - /* Skip over ',' (but not nul) */
> - if (*str)
> - str++;
> - }
> - return fecmode;
> -}
> -
> static int do_gfec(struct cmd_context *ctx)
> {
> struct ethtool_fecparam feccmd = { 0 };
> @@ -5041,22 +5008,26 @@ static int do_gfec(struct cmd_context *ctx)
>
> static int do_sfec(struct cmd_context *ctx)
> {
> - char *fecmode_str = NULL;
> + enum { ARG_NONE, ARG_ENCODING } state = ARG_NONE;
> struct ethtool_fecparam feccmd;
> - struct cmdline_info cmdline_fec[] = {
> - { "encoding", CMDL_STR, &fecmode_str, &feccmd.fec},
> - };
> - int changed;
> - int fecmode;
> - int rv;
> + int fecmode = 0, newmode;
> + int rv, i;
>
> - parse_generic_cmdline(ctx, &changed, cmdline_fec,
> - ARRAY_SIZE(cmdline_fec));
> -
> - if (!fecmode_str)
> + for (i = 0; i < ctx->argc; i++) {
> + if (!strcmp(ctx->argp[i], "encoding")) {
> + state = ARG_ENCODING;
> + continue;
> + }
> + if (state == ARG_ENCODING) {
> + newmode = fecmode_str_to_type(ctx->argp[i]);
> + if (!newmode)
> + exit_bad_args();
> + fecmode |= newmode;
> + continue;
> + }
> exit_bad_args();
> + }
>
> - fecmode = parse_fecmode(fecmode_str);
> if (!fecmode)
> exit_bad_args();
>
> @@ -5265,7 +5236,7 @@ static const struct option {
> " [ all ]\n"},
> { "--show-fec", 1, do_gfec, "Show FEC settings"},
> { "--set-fec", 1, do_sfec, "Set FEC settings",
> - " [ encoding auto|off|rs|baser ]\n"},
> + " [ encoding auto|off|rs|baser [...]]\n"},
> { "-h|--help", 0, show_usage, "Show this help" },
> { "--version", 0, do_version, "Show version number" },
> {}
> diff --git a/test-cmdline.c b/test-cmdline.c
> index 9c51cca..84630a5 100644
> --- a/test-cmdline.c
> +++ b/test-cmdline.c
> @@ -268,12 +268,12 @@ static struct test_case {
> { 1, "--set-eee devname advertise foo" },
> { 1, "--set-fec devname" },
> { 0, "--set-fec devname encoding auto" },
> - { 0, "--set-fec devname encoding off," },
> - { 0, "--set-fec devname encoding baser,rs" },
> - { 0, "--set-fec devname encoding auto,auto," },
> + { 0, "--set-fec devname encoding off" },
> + { 0, "--set-fec devname encoding baser rs" },
> + { 0, "--set-fec devname encoding auto auto" },
> { 1, "--set-fec devname encoding foo" },
> - { 1, "--set-fec devname encoding auto,foo" },
> - { 1, "--set-fec devname encoding auto,," },
> + { 1, "--set-fec devname encoding auto foo" },
> + { 1, "--set-fec devname encoding none" },
> { 1, "--set-fec devname auto" },
> /* can't test --set-priv-flags yet */
> { 0, "-h" },
^ permalink raw reply
* Re: [PATCH][next-next][v2] netlink: avoid to allocate full skb when sending to many devices
From: Eric Dumazet @ 2018-09-20 13:43 UTC (permalink / raw)
To: Li RongQing, netdev
In-Reply-To: <1537433690-24335-1-git-send-email-lirongqing@baidu.com>
On 09/20/2018 01:54 AM, Li RongQing wrote:
> if skb->head is vmalloc address, when this skb is delivered, full
> allocation for this skb is required, if there are many devices,
> the full allocation will be called for every devices
>
> now if it is vmalloc, allocate a new skb, whose data is not vmalloc
> address, and use new allocated skb to clone and send, to avoid to
> allocate full skb everytime.
>
> Signed-off-by: Zhang Yu <zhangyu31@baidu.com>
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
This looks a broken Signed-off-by chain to me.
Who really wrote this patch ?
Also, given that I gave feedback to your first version,
it is customary to CC me for future versions, unless you want me to not notice them.
And lastly this patch looks way too complicated to me.
You probably can write something much simpler.
Thanks.
^ permalink raw reply
* Re: [PATCH v2 2/2] hv_netvsc: pair VF based on serial number
From: Lorenzo Pieralisi @ 2018-09-20 13:43 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: kys, haiyangz, sthemmin, devel, netdev, linux-pci
In-Reply-To: <20180914195457.20433-3-sthemmin@microsoft.com>
On Fri, Sep 14, 2018 at 12:54:57PM -0700, Stephen Hemminger wrote:
> Matching network device based on MAC address is problematic
> since a non VF network device can be creted with a duplicate MAC
> address causing confusion and problems. The VMBus API does provide
> a serial number that is a better matching method.
>
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> ---
> drivers/net/hyperv/netvsc.c | 3 ++
> drivers/net/hyperv/netvsc_drv.c | 58 +++++++++++++++++++--------------
> 2 files changed, 36 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> index 31c3d77b4733..fe01e141c8f8 100644
> --- a/drivers/net/hyperv/netvsc.c
> +++ b/drivers/net/hyperv/netvsc.c
> @@ -1203,6 +1203,9 @@ static void netvsc_send_vf(struct net_device *ndev,
>
> net_device_ctx->vf_alloc = nvmsg->msg.v4_msg.vf_assoc.allocated;
> net_device_ctx->vf_serial = nvmsg->msg.v4_msg.vf_assoc.serial;
> + netdev_info(ndev, "VF slot %u %s\n",
> + net_device_ctx->vf_serial,
> + net_device_ctx->vf_alloc ? "added" : "removed");
> }
>
> static void netvsc_receive_inband(struct net_device *ndev,
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index 1121a1ec407c..9dedc1463e88 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -1894,20 +1894,6 @@ static void netvsc_link_change(struct work_struct *w)
> rtnl_unlock();
> }
>
> -static struct net_device *get_netvsc_bymac(const u8 *mac)
> -{
> - struct net_device_context *ndev_ctx;
> -
> - list_for_each_entry(ndev_ctx, &netvsc_dev_list, list) {
> - struct net_device *dev = hv_get_drvdata(ndev_ctx->device_ctx);
> -
> - if (ether_addr_equal(mac, dev->perm_addr))
> - return dev;
> - }
> -
> - return NULL;
> -}
> -
> static struct net_device *get_netvsc_byref(struct net_device *vf_netdev)
> {
> struct net_device_context *net_device_ctx;
> @@ -2036,26 +2022,48 @@ static void netvsc_vf_setup(struct work_struct *w)
> rtnl_unlock();
> }
>
> +/* Find netvsc by VMBus serial number.
> + * The PCI hyperv controller records the serial number as the slot.
> + */
> +static struct net_device *get_netvsc_byslot(const struct net_device *vf_netdev)
> +{
> + struct device *parent = vf_netdev->dev.parent;
> + struct net_device_context *ndev_ctx;
> + struct pci_dev *pdev;
> +
> + if (!parent || !dev_is_pci(parent))
> + return NULL; /* not a PCI device */
> +
> + pdev = to_pci_dev(parent);
> + if (!pdev->slot) {
> + netdev_notice(vf_netdev, "no PCI slot information\n");
> + return NULL;
> + }
> +
> + list_for_each_entry(ndev_ctx, &netvsc_dev_list, list) {
> + if (!ndev_ctx->vf_alloc)
> + continue;
> +
> + if (ndev_ctx->vf_serial == pdev->slot->number)
> + return hv_get_drvdata(ndev_ctx->device_ctx);
In patch 1, pdev->slot->number is set to:
PCI_SLOT(wslot_to_devfn(hpdev->desc.win_slot.slot))
so I assume vf_serial is initialized (I have no knowledge of VMBUS and
hyper-V internals) to that value, somehow.
I also do not know how the wslot stuff is handled but I assume 5 bits
(ie dev bits in devfn) are enough.
BTW, I have noticed this patch (and patch 1) are already in -next so I will
drop them from the PCI patch queue.
Lorenzo
> + }
> +
> + netdev_notice(vf_netdev,
> + "no netdev found for slot %u\n", pdev->slot->number);
> + return NULL;
> +}
> +
> static int netvsc_register_vf(struct net_device *vf_netdev)
> {
> - struct net_device *ndev;
> struct net_device_context *net_device_ctx;
> - struct device *pdev = vf_netdev->dev.parent;
> struct netvsc_device *netvsc_dev;
> + struct net_device *ndev;
> int ret;
>
> if (vf_netdev->addr_len != ETH_ALEN)
> return NOTIFY_DONE;
>
> - if (!pdev || !dev_is_pci(pdev) || dev_is_pf(pdev))
> - return NOTIFY_DONE;
> -
> - /*
> - * We will use the MAC address to locate the synthetic interface to
> - * associate with the VF interface. If we don't find a matching
> - * synthetic interface, move on.
> - */
> - ndev = get_netvsc_bymac(vf_netdev->perm_addr);
> + ndev = get_netvsc_byslot(vf_netdev);
> if (!ndev)
> return NOTIFY_DONE;
>
> --
> 2.18.0
>
^ permalink raw reply
* Re: [PATCH v2 2/4] dt-bindings: net: qcom: Add binding for shared mdio bus
From: Timur Tabi @ 2018-09-20 13:42 UTC (permalink / raw)
To: Andrew Lunn
Cc: Wang, Dongsheng, Florian Fainelli, davem@davemloft.net,
Zheng, Joey, netdev@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <20180919152052.GD17466@lunn.ch>
On 9/19/18 10:20 AM, Andrew Lunn wrote:
> I suspect that is not going to be easy. Last time i looked, the ACPI
> standard had nothing about MDIO busses or PHYs. Marcin Wojtas did some
> work in this area a while back for the mvpp2, but if i remember
> correctly, he worked around this by simply not having a PHY when using
> ACPI, and making use of a MAC interrupt which indicated when there was
> link.
>
> Whoever implements this first needs to be an ACPI expert and probably
> needs to write it up and submit it as an amendment to the ACPI
> standard.
If that's what it takes, then so be it. But adding DT support for a
device that is only used on ACPI platforms is not a worthwhile endeavor.
After ACPI support is merged, Dongsheng can choose to add DT support to
maintain parity, if he wants. Maybe one day the MSM developers will use
the upstream driver on one of their SOCs.
^ permalink raw reply
* Re: [PATCH bpf-next 2/3] bpf: emit RECORD_MMAP events for bpf prog load/unload
From: Peter Zijlstra @ 2018-09-20 13:36 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: David S . Miller, daniel, acme, netdev, kernel-team
In-Reply-To: <20180920084424.GN24124@hirez.programming.kicks-ass.net>
On Thu, Sep 20, 2018 at 10:44:24AM +0200, Peter Zijlstra wrote:
> On Wed, Sep 19, 2018 at 03:39:34PM -0700, Alexei Starovoitov wrote:
> > void bpf_prog_kallsyms_del(struct bpf_prog *fp)
> > {
> > + unsigned long symbol_start, symbol_end;
> > + /* mmap_record.filename cannot be NULL and has to be u64 aligned */
> > + char buf[sizeof(u64)] = {};
> > +
> > if (!bpf_prog_kallsyms_candidate(fp))
> > return;
> >
> > spin_lock_bh(&bpf_lock);
> > bpf_prog_ksym_node_del(fp->aux);
> > spin_unlock_bh(&bpf_lock);
> > + bpf_get_prog_addr_region(fp, &symbol_start, &symbol_end);
> > + perf_event_mmap_bpf_prog(symbol_start, symbol_end - symbol_start,
> > + buf, sizeof(buf));
> > }
>
> So perf doesn't normally issue unmap events.. We've talked about doing
> that, but so far it's never really need needed I think.
>
> I feels a bit weird to start issuing unmap events for this.
FWIW:
https://lkml.kernel.org/r/20170127130702.GI6515@twins.programming.kicks-ass.net
has talk of PERF_RECORD_MUNMAP
^ permalink raw reply
* Re: [PATCH perf 3/3] tools/perf: recognize and process RECORD_MMAP events for bpf progs
From: Arnaldo Carvalho de Melo @ 2018-09-20 13:36 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: David S . Miller, daniel, peterz, netdev, kernel-team
In-Reply-To: <20180919223935.999270-4-ast@kernel.org>
Em Wed, Sep 19, 2018 at 03:39:35PM -0700, Alexei Starovoitov escreveu:
> Recognize JITed bpf prog load/unload events.
> Add/remove kernel symbols accordingly.
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
> tools/perf/util/machine.c | 27 +++++++++++++++++++++++++++
> tools/perf/util/symbol.c | 13 +++++++++++++
> tools/perf/util/symbol.h | 1 +
> 3 files changed, 41 insertions(+)
>
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index c4acd2001db0..ae4f8a0fdc7e 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -25,6 +25,7 @@
> #include "sane_ctype.h"
> #include <symbol/kallsyms.h>
> #include <linux/mman.h>
> +#include <linux/magic.h>
>
> static void __machine__remove_thread(struct machine *machine, struct thread *th, bool lock);
>
> @@ -1460,6 +1461,32 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
> enum dso_kernel_type kernel_type;
> bool is_kernel_mmap;
>
> + /* process JITed bpf programs load/unload events */
> + if (event->mmap.pid == ~0u && event->mmap.tid == BPF_FS_MAGIC) {
So, this would be in machine__process_kernel_munmap-event(machine), etc,
no check for BPF_FS_MAGIC would be needed with a PERF_RECORD_MUNMAP.
> + struct symbol *sym;
> + u64 ip;
> +
> + map = map_groups__find(&machine->kmaps, event->mmap.start);
> + if (!map) {
> + pr_err("No kernel map for IP %lx\n", event->mmap.start);
> + goto out_problem;
> + }
> + ip = event->mmap.start - map->start + map->pgoff;
> + if (event->mmap.filename[0]) {
> + sym = symbol__new(ip, event->mmap.len, 0, 0,
> + event->mmap.filename);
Humm, so the bpf program would be just one symbol... bpf-to-bpf calls
will be to a different bpf program, right?
/me goes to read https://lwn.net/Articles/741773/
"[PATCH bpf-next 00/13] bpf: introduce function calls"
> + dso__insert_symbol(map->dso, sym);
> + } else {
> + if (symbols__erase(&map->dso->symbols, ip)) {
> + pr_err("No bpf prog at IP %lx/%lx\n",
> + event->mmap.start, ip);
> + goto out_problem;
> + }
> + dso__reset_find_symbol_cache(map->dso);
> + }
> + return 0;
> + }
> +
> /* If we have maps from kcore then we do not need or want any others */
> if (machine__uses_kcore(machine))
> return 0;
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index d188b7588152..0653f313661d 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -353,6 +353,19 @@ static struct symbol *symbols__find(struct rb_root *symbols, u64 ip)
> return NULL;
> }
>
> +int symbols__erase(struct rb_root *symbols, u64 ip)
> +{
> + struct symbol *s;
> +
> + s = symbols__find(symbols, ip);
> + if (!s)
> + return -ENOENT;
> +
> + rb_erase(&s->rb_node, symbols);
> + symbol__delete(s);
> + return 0;
> +}
> +
> static struct symbol *symbols__first(struct rb_root *symbols)
> {
> struct rb_node *n = rb_first(symbols);
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index f25fae4b5743..92ef31953d9a 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -310,6 +310,7 @@ char *dso__demangle_sym(struct dso *dso, int kmodule, const char *elf_name);
>
> void __symbols__insert(struct rb_root *symbols, struct symbol *sym, bool kernel);
> void symbols__insert(struct rb_root *symbols, struct symbol *sym);
> +int symbols__erase(struct rb_root *symbols, u64 ip);
> void symbols__fixup_duplicate(struct rb_root *symbols);
> void symbols__fixup_end(struct rb_root *symbols);
> void map_groups__fixup_end(struct map_groups *mg);
> --
> 2.17.1
^ permalink raw reply
* Re: [PATCH net] af_key: free SKBs under RCU protection
From: Eric Dumazet @ 2018-09-20 13:29 UTC (permalink / raw)
To: Sean Tranchetti, netdev, steffen.klassert
In-Reply-To: <1537402712-12875-1-git-send-email-stranche@codeaurora.org>
On 09/19/2018 05:18 PM, Sean Tranchetti wrote:
> pfkey_broadcast() can make calls to pfkey_broadcast_one() which
> will clone or copy the passed in SKB. This new SKB will be assigned
> the sock_rfree() function as its destructor, which requires that
> the socket reference the SKB contains is valid when the SKB is freed.
>
> If this SKB is ever passed to pfkey_broadcast() again by some other
> function (such as pkfey_dump() or pfkey_promisc) it will then be
> freed there. However, since this free occurs outside of RCU protection,
> it is possible that userspace could close the socket and trigger
> pfkey_release() to free the socket before sock_rfree() can run, creating
> the following race condition:
>
> 1: An SKB belonging to the pfkey socket is passed to pfkey_broadcast().
> It performs the broadcast to any other sockets, and calls
> rcu_read_unlock(), but does not yet reach kfree_skb().
> 2: Userspace closes the socket, triggering pfkey_realse(). Since no one
> holds the RCU lock, synchronize_rcu() returns and it is allowed to
> continue. It calls sock_put() to free the socket.
> 3: pfkey_broadcast() now calls kfree_skb() on the original SKB it was
> passed, triggering a call to sock_rfree(). This function now accesses
> the freed struct sock * via skb->sk, and attempts to update invalid
> memory.
>
> By ensuring that the pfkey_broadcast() also frees the SKBs while it holds
> the RCU lock, we can ensure that the socket will remain valid when the SKB
> is freed, avoiding crashes like the following:
>
> Unable to handle kernel paging request at virtual address 6b6b6b6b6b6c4b
> [006b6b6b6b6b6c4b] address between user and kernel address ranges
> Internal error: Oops: 96000004 [#1] PREEMPT SMP
> task: fffffff78f65b380 task.stack: ffffff8049a88000
> pc : sock_rfree+0x38/0x6c
> lr : skb_release_head_state+0x6c/0xcc
> Process repro (pid: 7117, stack limit = 0xffffff8049a88000)
> Call trace:
> sock_rfree+0x38/0x6c
> skb_release_head_state+0x6c/0xcc
> skb_release_all+0x1c/0x38
> __kfree_skb+0x1c/0x30
> kfree_skb+0xd0/0xf4
> pfkey_broadcast+0x14c/0x18c
> pfkey_sendmsg+0x1d8/0x408
> sock_sendmsg+0x44/0x60
> ___sys_sendmsg+0x1d0/0x2a8
> __sys_sendmsg+0x64/0xb4
> SyS_sendmsg+0x34/0x4c
> el0_svc_naked+0x34/0x38
> Kernel panic - not syncing: Fatal exception
>
> Signed-off-by: Sean Tranchetti <stranche@codeaurora.org>
> ---
> net/key/af_key.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/key/af_key.c b/net/key/af_key.c
> index 9d61266..dd257c7 100644
> --- a/net/key/af_key.c
> +++ b/net/key/af_key.c
> @@ -275,13 +275,13 @@ static int pfkey_broadcast(struct sk_buff *skb, gfp_t allocation,
> if ((broadcast_flags & BROADCAST_REGISTERED) && err)
> err = err2;
> }
> - rcu_read_unlock();
>
> if (one_sk != NULL)
> err = pfkey_broadcast_one(skb, &skb2, allocation, one_sk);
>
> kfree_skb(skb2);
> kfree_skb(skb);
> + rcu_read_unlock();
> return err;
> }
>
>
I do not believe the changelog or the patch makes sense.
Having skb still referencing a socket prevents this socket being released.
If you think about it, what would prevent the freeing happening
_before_ the rcu_read_lock() in pfkey_broadcast() ?
Maybe the correct fix is that pfkey_broadcast_one() should ensure the socket is still valid.
I would suggest something like :
diff --git a/net/key/af_key.c b/net/key/af_key.c
index 9d61266526e767770d9a1ce184ac8cdd59de309a..5ce309d020dda5e46e4426c4a639bfb551e2260d 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -201,7 +201,9 @@ static int pfkey_broadcast_one(struct sk_buff *skb, struct sk_buff **skb2,
{
int err = -ENOBUFS;
- sock_hold(sk);
+ if (!refcount_inc_not_zero(&sk->sk_refcnt))
+ return -ENOENT;
+
if (*skb2 == NULL) {
if (refcount_read(&skb->users) != 1) {
*skb2 = skb_clone(skb, allocation);
^ 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