* Re: [v2 PATCH -tip 3/6] net: sctp: Add SCTP ACK tracking trace event
From: Steven Rostedt @ 2017-12-18 17:05 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Ingo Molnar, Ian McDonald, Vlad Yasevich, Stephen Hemminger,
Peter Zijlstra, Thomas Gleixner, LKML, H . Peter Anvin,
Gerrit Renker, David S . Miller, Neil Horman, dccp, netdev,
linux-sctp, Stephen Rothwell
In-Reply-To: <151358473510.28850.10475072993963389604.stgit@devbox>
On Mon, 18 Dec 2017 17:12:15 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:
> Add SCTP ACK tracking trace event to trace the changes of SCTP
> association state in response to incoming packets.
> It is used for debugging SCTP congestion control algorithms,
> and will replace sctp_probe module.
>
> Note that this event a bit tricky. Since this consists of 2
> events (sctp_probe and sctp_probe_path) so you have to enable
> both events as below.
>
> # cd /sys/kernel/debug/tracing
> # echo 1 > events/sctp/sctp_probe/enable
> # echo 1 > events/sctp/sctp_probe_path/enable
>
> Or, you can enable all the events under sctp.
>
> # echo 1 > events/sctp/enable
>
> Since sctp_probe_path event is always invoked from sctp_probe
> event, you can not see any output if you only enable
> sctp_probe_path.
I have to ask, why did you do it this way?
> +#include <trace/define_trace.h>
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index 8f8ccded13e4..c5f92b2cc5c3 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -59,6 +59,9 @@
> #include <net/sctp/sm.h>
> #include <net/sctp/structs.h>
>
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/sctp.h>
> +
> static struct sctp_packet *sctp_abort_pkt_new(
> struct net *net,
> const struct sctp_endpoint *ep,
> @@ -3219,6 +3222,8 @@ enum sctp_disposition sctp_sf_eat_sack_6_2(struct net *net,
> struct sctp_sackhdr *sackh;
> __u32 ctsn;
>
> + trace_sctp_probe(ep, asoc, chunk);
What about doing this right after this probe:
if (trace_sctp_probe_path_enabled()) {
struct sctp_transport *sp;
list_for_each_entry(sp, &asoc->peer.transpor_addr_list,
transports) {
trace_sctp_probe_path(sp, asoc);
}
}
The "trace_sctp_probe_path_enabled()" is a static branch, which means
it's a nop just like a tracepoint is, and will not add any overhead if
the trace_sctp_probe_path is not enabled.
-- Steve
> +
> if (!sctp_vtag_verify(chunk, asoc))
> return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
>
^ permalink raw reply
* RE: [Intel-wired-lan] v4.15-rc2 on thinkpad x60: ethernet stopped working
From: Fujinaka, Todd @ 2017-12-18 17:07 UTC (permalink / raw)
To: Neftin, Sasha, Pavel Machek, Keller, Jacob E
Cc: bpoirier@suse.com, nix.or.die@gmail.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, intel-wired-lan@lists.osuosl.org,
lsorense@csclub.uwaterloo.ca, David Miller
In-Reply-To: <077087f2-551a-c045-6b07-b1b661e53dad@intel.com>
Jeff was out sick last week. It might take him a bit to catch up.
I'll remind him when I see him next (which I hope is soon).
Todd Fujinaka
Software Application Engineer
Datacenter Engineering Group
Intel Corporation
todd.fujinaka@intel.com
-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf Of Neftin, Sasha
Sent: Monday, December 18, 2017 7:50 AM
To: Pavel Machek <pavel@ucw.cz>; Keller, Jacob E <jacob.e.keller@intel.com>
Cc: bpoirier@suse.com; nix.or.die@gmail.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; intel-wired-lan@lists.osuosl.org; lsorense@csclub.uwaterloo.ca; David Miller <davem@davemloft.net>
Subject: Re: [Intel-wired-lan] v4.15-rc2 on thinkpad x60: ethernet stopped working
On 12/18/2017 13:58, Pavel Machek wrote:
> On Mon 2017-12-18 13:24:40, Neftin, Sasha wrote:
>> On 12/18/2017 12:26, Pavel Machek wrote:
>>> Hi!
>>>
>>>>>>> In v4.15-rc2+, network manager can not see my ethernet card, and
>>>>>>> manual attempts to ifconfig it up did not really help, either.
>>>>>>>
>>>>>>> Card is:
>>>>>>>
>>>>>>> 02:00.0 Ethernet controller: Intel Corporation 82573L Gigabit
>>>>>>> Ethernet Controller
>>>>> ....
>>>>>>> Any ideas ?
>>>>>> Yes , 19110cfbb34d4af0cdfe14cd243f3b09dc95b013 broke it.
>>>>>>
>>>>>> See:
>>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=198047
>>>>>>
>>>>>> Fix there :
>>>>>> https://marc.info/?l=linux-kernel&m=151272209903675&w=2
>>>>> I don't see the patch in latest mainline. Not having ethernet
>>>>> is... somehow annoying. What is going on there?
>>>> Generally speaking, e1000 maintainence has been handled very poorly
>>>> over the past few years, I have to say.
>>>>
>>>> Fixes take forever to propagate even when someone other than the
>>>> maintainer provides a working and tested fix, just like this case.
>>>>
>>>> Jeff, please take e1000 maintainence seriously and get these
>>>> critical bug fixes propagated.
>>> No response AFAICT. I guess I should test reverting
>>> 19110cfbb34d4af0cdfe14cd243f3b09dc95b013, then ask you for revert?
>> Hello Pavel,
>>
>> Before ask for reverting 19110cfbb..., please, check if follow patch
>> of Benjamin work for you http://patchwork.ozlabs.org/patch/846825/
> Jacob said, in another email:
>
> # Digging into this, the problem is complicated. The original bug #
> assumed behavior of the .check_for_link call, which is universally not
> # implemented.
> #
> # I think the correct fix is to revert 19110cfbb34d ("e1000e: Separate
> # signaling for link check/link up", 2017-10-10) and find a more proper solution.
>
> ...which makes me think that revert is preffered?
>
> Pavel
>
Pavel, before ask for revert - let's check Benjamin's patch following to his previous patch. Previous patch was not competed and latest one come to complete changes.
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply
* Re: [PATCH v3 net-next 0/6] tls: Add generic NIC offload infrastructure
From: Jiri Pirko @ 2017-12-18 17:10 UTC (permalink / raw)
To: Ilya Lesokhin
Cc: netdev, davem, davejwatson, tom, hannes, borisp, aviadye, liranl
In-Reply-To: <20171218111033.13256-1-ilyal@mellanox.com>
Mon, Dec 18, 2017 at 12:10:27PM CET, ilyal@mellanox.com wrote:
>Changes from v2:
>- Fix sk use after free and possible netdev use after free
>- tls device now keeps a refernce on the offloading netdev
>- tls device registers to the netdev notifer.
> Upon a NETDEV_DOWN event, offload is stopped and
> the reference on the netdev is dropped.
>- SW fallback support for skb->ip_summed != CHECKSUM_PARTIAL
>- Merged TLS patches are no longer part of this series.
>
>Changes from v1:
>- Remove the binding of the socket to a specific netdev
> through sk->sk_bound_dev_if.
> Add a check in validate_xmit_skb to detect route changes
> and call SW fallback code to do the crypto in software.
>- tls_get_record now returns the tls record sequence number.
> This is required to support connections with rcd_sn != iv.
>- Bug fixes to the TLS code.
>
>This patchset adds a generic infrastructure to offload TLS crypto to a
>network devices.
>
>patches 1-2 Export functions that we need
>patch 3 adds infrastructue for offloaded socket fallback
>patches 4-5 add new NDOs and capabilities.
>patch 6 adds the TLS NIC offload infrastructure.
>
>Github with mlx5e TLS offload support:
>https://github.com/Mellanox/tls-offload/tree/tls_device_v3
I don't get it. You are pushing infra but not the actual driver part
who is consuming the infra? Why?
^ permalink raw reply
* Re: BUG: unable to handle kernel NULL pointer dereference in rds_send_xmit
From: David Miller @ 2017-12-18 17:12 UTC (permalink / raw)
To: santosh.shilimkar
Cc: bot+aaf54a8c644d559d34dedcf3126aac68a20c9e63, linux-kernel,
linux-rdma, netdev, rds-devel, syzkaller-bugs
In-Reply-To: <5ba83a68-0103-d514-1b22-900f755f5aa4@oracle.com>
From: Santosh Shilimkar <santosh.shilimkar@oracle.com>
Date: Mon, 18 Dec 2017 08:28:05 -0800
> On 12/18/2017 12:43 AM, syzbot wrote:
>> Hello,
>> syzkaller hit the following crash on
>> 6084b576dca2e898f5c101baef151f7bfdbb606d
>> git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/master
>> compiler: gcc (GCC) 7.1.1 20170620
>> .config is attached
>> Raw console output is attached.
>> Unfortunately, I don't have any reproducer for this bug yet.
>> BUG: unable to handle kernel NULL pointer dereference at
>> 0000000000000028
>> program syz-executor6 is using a deprecated SCSI ioctl, please convert
>> it to SG_IO
>> IP: rds_send_xmit+0x80/0x930 net/rds/send.c:186
>
> Looks like another one tripping on empty transport. Mostly below
> should
> address it but we will test it if it does.
>
> diff --git a/net/rds/send.c b/net/rds/send.c
> index 7244d2e..e2d0eaa 100644
> --- a/net/rds/send.c
> +++ b/net/rds/send.c
> @@ -183,7 +183,7 @@ int rds_send_xmit(struct rds_conn_path *cp)
> goto out;
> }
>
> - if (conn->c_trans->xmit_path_prepare)
> + if (conn->c_trans && conn->c_trans->xmit_path_prepare)
> conn->c_trans->xmit_path_prepare(cp);
We're seeming to accumulate a lot of checks like this, maybe there
is a more general way to deal with this problem?
^ permalink raw reply
* [net 1/1] tipc: remove leaving group member from all lists
From: Jon Maloy @ 2017-12-18 17:13 UTC (permalink / raw)
To: davem, netdev
Cc: mohan.krishna.ghanta.krishnamurthy, tung.q.nguyen, hoang.h.le,
jon.maloy, canh.d.luu, ying.xue, tipc-discussion
A group member going into state LEAVING should never go back to any
other state before it is finally deleted. However, this might happen
if the socket needs to send out a RECLAIM message during this interval.
Since we forget to remove the leaving member from the group's 'active'
or 'pending' list, the member might be selected for reclaiming, change
state to RECLAIMING, and get stuck in this state instead of being
deleted. This might lead to suppression of the expected 'member down'
event to the receiver.
We fix this by removing the member from all lists, except the RB tree,
at the moment it goes into state LEAVING.
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/group.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/tipc/group.c b/net/tipc/group.c
index efb5714..b96ec42 100644
--- a/net/tipc/group.c
+++ b/net/tipc/group.c
@@ -699,6 +699,9 @@ void tipc_group_proto_rcv(struct tipc_group *grp, bool *usr_wakeup,
if (!m)
return;
m->bc_syncpt = msg_grp_bc_syncpt(hdr);
+ list_del_init(&m->list);
+ list_del_init(&m->congested);
+ *usr_wakeup = true;
/* Wait until WITHDRAW event is received */
if (m->state != MBR_LEAVING) {
@@ -710,8 +713,6 @@ void tipc_group_proto_rcv(struct tipc_group *grp, bool *usr_wakeup,
ehdr = buf_msg(m->event_msg);
msg_set_grp_bc_seqno(ehdr, m->bc_syncpt);
__skb_queue_tail(inputq, m->event_msg);
- *usr_wakeup = true;
- list_del_init(&m->congested);
return;
case GRP_ADV_MSG:
if (!m)
@@ -863,6 +864,7 @@ void tipc_group_member_evt(struct tipc_group *grp,
msg_set_grp_bc_seqno(hdr, m->bc_rcv_nxt);
__skb_queue_tail(inputq, skb);
}
+ list_del_init(&m->list);
list_del_init(&m->congested);
}
*sk_rcvbuf = tipc_group_rcvbuf_limit(grp);
--
2.1.4
^ permalink raw reply related
* Re: Linux 4.14 - regression: broken tun/tap / bridge network with virtio - bisected
From: Andreas Hartmann @ 2017-12-18 17:11 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Michal Kubecek, Jason Wang, David Miller, Network Development
In-Reply-To: <CAF=yD-LWyCD4Y0aJ9O0e_CHLR+3JOeKicRRTEVCPxgw4XOcqGQ@mail.gmail.com>
On 12/17/2017 at 11:33 PM Willem de Bruijn wrote:
> On Fri, Dec 15, 2017 at 1:05 AM, Andreas Hartmann
> <andihartmann@01019freenet.de> wrote:
>> On 12/14/2017 at 11:17 PM Willem de Bruijn wrote:
>>>>> Well, the patch does not fix hanging VMs, which have been shutdown and
>>>>> can't be killed any more.
>>>>> Because of the stack trace
>>>>>
>>>>> [<ffffffffc0d0e3c5>] vhost_net_ubuf_put_and_wait+0x35/0x60 [vhost_net]
>>>>> [<ffffffffc0d0f264>] vhost_net_ioctl+0x304/0x870 [vhost_net]
>>>>> [<ffffffff9b25460f>] do_vfs_ioctl+0x8f/0x5c0
>>>>> [<ffffffff9b254bb4>] SyS_ioctl+0x74/0x80
>>>>> [<ffffffff9b00365b>] do_syscall_64+0x5b/0x100
>>>>> [<ffffffff9b78e7ab>] entry_SYSCALL64_slow_path+0x25/0x25
>>>>> [<ffffffffffffffff>] 0xffffffffffffffff
>>>>>
>>>>> I was hoping, that the problems could be related - but that seems not to
>>>>> be true.
>>>>
>>>> However, it turned out, that reverting the complete patchset "Remove UDP
>>>> Fragmentation Offload support" prevent hanging qemu processes.
>>>
>>> That implies a combination of UFO and vhost zerocopy. Disabling
>>> experimental_zcopytx in vhost_net will probably work around the bug
>>> then.
>
> I have been able to reproduce the hang by sending a UFO packet
> between two guests running v4.13 on a host running v4.15-rc1.
>
> The vhost_net_ubuf_ref refcount indeed hits overflow (-1) from
> vhost_zerocopy_callback being called for each segment of a
> segmented UFO skb. This refcount is decremented then on each
> segment, but incremented only once for the entire UFO skb.
>
> Before v4.14, these packets would be converted in skb_segment to
> regular copy packets with skb_orphan_frags and the callback function
> called once at this point. v4.14 added support for reference counted
> zerocopy skb that can pass through skb_orphan_frags unmodified and
> have their zerocopy state safely cloned with skb_zerocopy_clone.
>
> The call to skb_zerocopy_clone must come after skb_orphan_frags
> to limit cloning of this state to those skbs that can do so safely.
>
> Please try a host with the following patch. This fixes it for me. I intend to
> send it to net.
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index a592ca025fc4..d2d985418819 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -3654,8 +3654,6 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
>
> skb_shinfo(nskb)->tx_flags |= skb_shinfo(head_skb)->tx_flags &
> SKBTX_SHARED_FRAG;
> - if (skb_zerocopy_clone(nskb, head_skb, GFP_ATOMIC))
> - goto err;
>
> while (pos < offset + len) {
> if (i >= nfrags) {
> @@ -3681,6 +3679,8 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
>
> if (unlikely(skb_orphan_frags(frag_skb, GFP_ATOMIC)))
> goto err;
> + if (skb_zerocopy_clone(nskb, frag_skb, GFP_ATOMIC))
> + goto err;
>
> *nskb_frag = *frag;
> __skb_frag_ref(nskb_frag);
>
>
> This is relatively inefficient, as it calls skb_zerocopy_clone for each frag
> in the frags[] array. I will follow-up with a patch to net-next that only
> checks once per skb:
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 466581cf4cdc..a293a33604ec 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -3662,7 +3662,8 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
>
> skb_shinfo(nskb)->tx_flags |= skb_shinfo(head_skb)->tx_flags &
> SKBTX_SHARED_FRAG;
> - if (skb_zerocopy_clone(nskb, head_skb, GFP_ATOMIC))
> + if (skb_orphan_frags(frag_skb, GFP_ATOMIC) ||
> + skb_zerocopy_clone(nskb, frag_skb, GFP_ATOMIC))
> goto err;
>
> while (pos < offset + len) {
> @@ -3676,6 +3677,11 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
>
> BUG_ON(!nfrags);
>
> + if (skb_orphan_frags(frag_skb, GFP_ATOMIC) ||
> + skb_zerocopy_clone(nskb, frag_skb,
> + GFP_ATOMIC))
> + goto err;
> +
> list_skb = list_skb->next;
> }
>
> @@ -3687,9 +3693,6 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
> goto err;
> }
>
> - if (unlikely(skb_orphan_frags(frag_skb, GFP_ATOMIC)))
> - goto err;
> -
I'm currently testing this one.
>
> I'll also send to net-next
>
> (1) a patch to convert its vhost_net_ ubuf_ref refcnt to refcount_t
>
> (2) a path to skb_zerocopy_clone to warn on clone if not
> sock_zerocopy_callback
>
>> I already tested it w/ options vhost_net experimental_zcopytx=0 - but
>> this didn't "resolve" anything. See
>> https://www.mail-archive.com/netdev@vger.kernel.org/msg203197.html
>>
>> Therefore, I think your following thoughts are lapsed unfortunately,
>> aren't they?
>
> That experiment was perhaps run before commit 0c19f846d582 ("net:
> accept UFO datagrams from tuntap and packet") and hit the other UFO
> bug.
That's probably true.
Thanks,
Andreas
^ permalink raw reply
* Re: BUG: unable to handle kernel NULL pointer dereference in rds_send_xmit
From: Santosh Shilimkar @ 2017-12-18 17:16 UTC (permalink / raw)
To: David Miller
Cc: bot+aaf54a8c644d559d34dedcf3126aac68a20c9e63-Pl5Pbv+GP7P466ipTTIvnc23WoclnBCfAL8bYrjMMd8,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
syzkaller-bugs-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20171218.121213.289437104214632276.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On 12/18/2017 9:12 AM, David Miller wrote:
> From: Santosh Shilimkar <santosh.shilimkar-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Date: Mon, 18 Dec 2017 08:28:05 -0800
>
>> On 12/18/2017 12:43 AM, syzbot wrote:
>>> Hello,
>>> syzkaller hit the following crash on
>>> 6084b576dca2e898f5c101baef151f7bfdbb606d
>>> git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/master
>>> compiler: gcc (GCC) 7.1.1 20170620
>>> .config is attached
>>> Raw console output is attached.
>>> Unfortunately, I don't have any reproducer for this bug yet.
>>> BUG: unable to handle kernel NULL pointer dereference at
>>> 0000000000000028
>>> program syz-executor6 is using a deprecated SCSI ioctl, please convert
>>> it to SG_IO
>>> IP: rds_send_xmit+0x80/0x930 net/rds/send.c:186
>>
>> Looks like another one tripping on empty transport. Mostly below
>> should
>> address it but we will test it if it does.
>>
>> diff --git a/net/rds/send.c b/net/rds/send.c
>> index 7244d2e..e2d0eaa 100644
>> --- a/net/rds/send.c
>> +++ b/net/rds/send.c
>> @@ -183,7 +183,7 @@ int rds_send_xmit(struct rds_conn_path *cp)
>> goto out;
>> }
>>
>> - if (conn->c_trans->xmit_path_prepare)
>> + if (conn->c_trans && conn->c_trans->xmit_path_prepare)
>> conn->c_trans->xmit_path_prepare(cp);
>
> We're seeming to accumulate a lot of checks like this, maybe there
> is a more general way to deal with this problem?
>
Agree. Some of these additional transports hooks got added later
to specific transports which needs them. Will review this overall
and see if it can be addressed generically.
Regards,
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [rds-devel] BUG: unable to handle kernel NULL pointer dereference in rds_send_xmit
From: Sowmini Varadhan @ 2017-12-18 17:22 UTC (permalink / raw)
To: David Miller
Cc: santosh.shilimkar, rds-devel,
bot+aaf54a8c644d559d34dedcf3126aac68a20c9e63, linux-rdma, netdev,
syzkaller-bugs, linux-kernel
In-Reply-To: <20171218.121213.289437104214632276.davem@davemloft.net>
> From: Santosh Shilimkar <santosh.shilimkar@oracle.com>
> Date: Mon, 18 Dec 2017 08:28:05 -0800
:
> > Looks like another one tripping on empty transport. Mostly below
> > should
> > address it but we will test it if it does.
that was my first thought, but it cannot be the case here: rds_sendmsg
etc itself would have bombed if that were the case, and the packet
would never have gotten queued.
This is unlike f3069c6d33, where an applications skips the transport
binding (either misses the explicit bind, or gets the wrong transport
due to an implicit bind) before it triggers the setsockopt.
I suspect that the problems is that the conn (and thus c_trans)
have gotten destroyed, but the cp_send_w work got incorrectly
re-queued. For example, rds_cong_queue_updates() (because the
peer sent a congestion update) can happen in softirq context,
and would end up requeing work in the middle of rds_conn_destroy,
after we have assumed that everything is quisced.
On (12/18/17 12:12), David Miller wrote:
>
> We're seeming to accumulate a lot of checks like this, maybe there
> is a more general way to deal with this problem?
Yeah, I was thinking about this.. let me try to reprodcue this in-house
and get back with a patchset.
--Sowmini
^ permalink raw reply
* Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program
From: Jason Gunthorpe @ 2017-12-18 17:46 UTC (permalink / raw)
To: Joe Perches
Cc: Knut Omang, Stephen Hemminger,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Mauro Carvalho Chehab,
Nicolas Palix, Jonathan Corbet, Santosh Shilimkar, Matthew Wilcox,
cocci-/FJkirnvOdkvYVN+rsErww, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA, Doug Ledford,
Mickaël Salaün, Shuah Khan,
linux-kbuild-u79uwXL29TY76Z2rM5mHXA, Michal Marek, Julia Lawall,
John Haxby, Åsmund Østvold
In-Reply-To: <1513576817.31581.58.camel-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
On Sun, Dec 17, 2017 at 10:00:17PM -0800, Joe Perches wrote:
> > Today when we run checkers we get so many warnings it is too hard to
> > make any sense of it.
>
> Here is a list of the checkpatch messages for drivers/infiniband
> sorted by type.
>
> Many of these might be corrected by using
>
> $ ./scripts/checkpatch.pl -f --fix-inplace --types=<TYPE> \
> $(git ls-files drivers/infiniband/)
How many of these do you think it is worth to fix?
We do get a steady trickle of changes in this topic every cycle.
Is it better to just do a big number of them all at once? Do you have
an idea how disruptive this kind of work is to the whole patch flow
eg new patches no longer applying to for-next, backports no longer
applying, merge conflicts?
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH][next] bpf: make function skip_callee static and return NULL rather than 0
From: Colin King @ 2017-12-18 17:47 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, netdev; +Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Function skip_callee is local to the source and does not need to
be in global scope, so make it static. Also return NULL rather than 0.
Cleans up two sparse warnings:
symbol 'skip_callee' was not declared. Should it be static?
Using plain integer as NULL pointer
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
kernel/bpf/verifier.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 2f6f09cd1925..52689f2abbcb 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -823,6 +823,7 @@ static int check_subprogs(struct bpf_verifier_env *env)
return 0;
}
+static
struct bpf_verifier_state *skip_callee(struct bpf_verifier_env *env,
const struct bpf_verifier_state *state,
struct bpf_verifier_state *parent,
@@ -867,7 +868,7 @@ struct bpf_verifier_state *skip_callee(struct bpf_verifier_env *env,
verbose(env, "verifier bug regno %d tmp %p\n", regno, tmp);
verbose(env, "regno %d parent frame %d current frame %d\n",
regno, parent->curframe, state->curframe);
- return 0;
+ return NULL;
}
static int mark_reg_read(struct bpf_verifier_env *env,
--
2.14.1
^ permalink raw reply related
* [PATCH iproute2 0/3] ip/tunnels: Reuse code, vti6 zero endpoint support and minor cleanup
From: Serhey Popovych @ 2017-12-18 17:48 UTC (permalink / raw)
To: netdev
In this series I present next set of improvements:
1) Use tnl_parse_key() to avoid code duplication in tunnel
configuration via netlink code.
2) Trivial: use IN6ADDR_ANY_INIT instead of open coded
initialization of local/remote endpoint in ip6tnl code.
3) Trivial: drop additional checks for zero endpoint
in vti6 code. This completes and unifies support for
unconfiguring local/remote endpoint for tunnel.
See individual patch description message for details.
Thanks,
Serhii
Serhey Popovych (3):
ip/tunnel: Use tnl_parse_key() to parse tunnel key
link_ip6tnl: Use IN6ADDR_ANY_INIT to initialize local/remote
endpoints
link_vti6: Always add local/remote endpoint attributes
ip/link_gre.c | 45 +++++----------------------------------------
ip/link_gre6.c | 45 +++++----------------------------------------
ip/link_ip6tnl.c | 4 ++--
ip/link_vti.c | 45 +++++----------------------------------------
ip/link_vti6.c | 51 +++++++--------------------------------------------
ip/tunnel.c | 5 +++--
6 files changed, 27 insertions(+), 168 deletions(-)
--
1.7.10.4
^ permalink raw reply
* [PATCH iproute2 1/3] ip/tunnel: Use tnl_parse_key() to parse tunnel key
From: Serhey Popovych @ 2017-12-18 17:48 UTC (permalink / raw)
To: netdev
In-Reply-To: <1513619285-23187-1-git-send-email-serhe.popovych@gmail.com>
It is added with commit a7ed1520ee96 (ip/tunnel:
introduce tnl_parse_key()) to avoid code duplication
in ip6?tunnel.c.
Reuse it for gre/gre6 and vti/vti6 tunnel rtnl
configuration interface with the same purpose
it is used in tunnel ioctl interface in ip6?tunnel.c.
While there change type of key variables from
unsigned integer to __be32 to reflect nature of the
value they store and place error message in
tnl_parse_key() on a single line to make single
call to fprintf().
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/link_gre.c | 45 +++++----------------------------------------
ip/link_gre6.c | 45 +++++----------------------------------------
ip/link_vti.c | 45 +++++----------------------------------------
ip/link_vti6.c | 45 +++++----------------------------------------
ip/tunnel.c | 5 +++--
5 files changed, 23 insertions(+), 162 deletions(-)
diff --git a/ip/link_gre.c b/ip/link_gre.c
index 09f1e44..2397920 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -81,8 +81,8 @@ static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
struct rtattr *greinfo[IFLA_GRE_MAX + 1];
__u16 iflags = 0;
__u16 oflags = 0;
- unsigned int ikey = 0;
- unsigned int okey = 0;
+ __be32 ikey = 0;
+ __be32 okey = 0;
unsigned int saddr = 0;
unsigned int daddr = 0;
unsigned int link = 0;
@@ -184,53 +184,18 @@ get_failed:
while (argc > 0) {
if (!matches(*argv, "key")) {
- unsigned int uval;
-
NEXT_ARG();
iflags |= GRE_KEY;
oflags |= GRE_KEY;
- if (strchr(*argv, '.'))
- uval = get_addr32(*argv);
- else {
- if (get_unsigned(&uval, *argv, 0) < 0) {
- fprintf(stderr,
- "Invalid value for \"key\": \"%s\"; it should be an unsigned integer\n", *argv);
- exit(-1);
- }
- uval = htonl(uval);
- }
-
- ikey = okey = uval;
+ ikey = okey = tnl_parse_key("key", *argv);
} else if (!matches(*argv, "ikey")) {
- unsigned int uval;
-
NEXT_ARG();
iflags |= GRE_KEY;
- if (strchr(*argv, '.'))
- uval = get_addr32(*argv);
- else {
- if (get_unsigned(&uval, *argv, 0) < 0) {
- fprintf(stderr, "invalid value for \"ikey\": \"%s\"; it should be an unsigned integer\n", *argv);
- exit(-1);
- }
- uval = htonl(uval);
- }
- ikey = uval;
+ ikey = tnl_parse_key("ikey", *argv);
} else if (!matches(*argv, "okey")) {
- unsigned int uval;
-
NEXT_ARG();
oflags |= GRE_KEY;
- if (strchr(*argv, '.'))
- uval = get_addr32(*argv);
- else {
- if (get_unsigned(&uval, *argv, 0) < 0) {
- fprintf(stderr, "invalid value for \"okey\": \"%s\"; it should be an unsigned integer\n", *argv);
- exit(-1);
- }
- uval = htonl(uval);
- }
- okey = uval;
+ okey = tnl_parse_key("okey", *argv);
} else if (!matches(*argv, "seq")) {
iflags |= GRE_SEQ;
oflags |= GRE_SEQ;
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index c22fded..7190ada 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -92,8 +92,8 @@ static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
struct rtattr *greinfo[IFLA_GRE_MAX + 1];
__u16 iflags = 0;
__u16 oflags = 0;
- unsigned int ikey = 0;
- unsigned int okey = 0;
+ __be32 ikey = 0;
+ __be32 okey = 0;
struct in6_addr raddr = IN6ADDR_ANY_INIT;
struct in6_addr laddr = IN6ADDR_ANY_INIT;
unsigned int link = 0;
@@ -192,53 +192,18 @@ get_failed:
while (argc > 0) {
if (!matches(*argv, "key")) {
- unsigned int uval;
-
NEXT_ARG();
iflags |= GRE_KEY;
oflags |= GRE_KEY;
- if (strchr(*argv, '.'))
- uval = get_addr32(*argv);
- else {
- if (get_unsigned(&uval, *argv, 0) < 0) {
- fprintf(stderr,
- "Invalid value for \"key\"\n");
- exit(-1);
- }
- uval = htonl(uval);
- }
-
- ikey = okey = uval;
+ ikey = okey = tnl_parse_key("key", *argv);
} else if (!matches(*argv, "ikey")) {
- unsigned int uval;
-
NEXT_ARG();
iflags |= GRE_KEY;
- if (strchr(*argv, '.'))
- uval = get_addr32(*argv);
- else {
- if (get_unsigned(&uval, *argv, 0) < 0) {
- fprintf(stderr, "invalid value of \"ikey\"\n");
- exit(-1);
- }
- uval = htonl(uval);
- }
- ikey = uval;
+ ikey = tnl_parse_key("ikey", *argv);
} else if (!matches(*argv, "okey")) {
- unsigned int uval;
-
NEXT_ARG();
oflags |= GRE_KEY;
- if (strchr(*argv, '.'))
- uval = get_addr32(*argv);
- else {
- if (get_unsigned(&uval, *argv, 0) < 0) {
- fprintf(stderr, "invalid value of \"okey\"\n");
- exit(-1);
- }
- uval = htonl(uval);
- }
- okey = uval;
+ okey = tnl_parse_key("okey", *argv);
} else if (!matches(*argv, "seq")) {
iflags |= GRE_SEQ;
oflags |= GRE_SEQ;
diff --git a/ip/link_vti.c b/ip/link_vti.c
index 05aefa3..6c5469f 100644
--- a/ip/link_vti.c
+++ b/ip/link_vti.c
@@ -64,8 +64,8 @@ static int vti_parse_opt(struct link_util *lu, int argc, char **argv,
struct rtattr *tb[IFLA_MAX + 1];
struct rtattr *linkinfo[IFLA_INFO_MAX+1];
struct rtattr *vtiinfo[IFLA_VTI_MAX + 1];
- unsigned int ikey = 0;
- unsigned int okey = 0;
+ __be32 ikey = 0;
+ __be32 okey = 0;
unsigned int saddr = 0;
unsigned int daddr = 0;
unsigned int link = 0;
@@ -122,49 +122,14 @@ get_failed:
while (argc > 0) {
if (!matches(*argv, "key")) {
- unsigned int uval;
-
NEXT_ARG();
- if (strchr(*argv, '.'))
- uval = get_addr32(*argv);
- else {
- if (get_unsigned(&uval, *argv, 0) < 0) {
- fprintf(stderr,
- "Invalid value for \"key\": \"%s\"; it should be an unsigned integer\n", *argv);
- exit(-1);
- }
- uval = htonl(uval);
- }
-
- ikey = okey = uval;
+ ikey = okey = tnl_parse_key("key", *argv);
} else if (!matches(*argv, "ikey")) {
- unsigned int uval;
-
NEXT_ARG();
- if (strchr(*argv, '.'))
- uval = get_addr32(*argv);
- else {
- if (get_unsigned(&uval, *argv, 0) < 0) {
- fprintf(stderr, "invalid value for \"ikey\": \"%s\"; it should be an unsigned integer\n", *argv);
- exit(-1);
- }
- uval = htonl(uval);
- }
- ikey = uval;
+ ikey = tnl_parse_key("ikey", *argv);
} else if (!matches(*argv, "okey")) {
- unsigned int uval;
-
NEXT_ARG();
- if (strchr(*argv, '.'))
- uval = get_addr32(*argv);
- else {
- if (get_unsigned(&uval, *argv, 0) < 0) {
- fprintf(stderr, "invalid value for \"okey\": \"%s\"; it should be an unsigned integer\n", *argv);
- exit(-1);
- }
- uval = htonl(uval);
- }
- okey = uval;
+ okey = tnl_parse_key("okey", *argv);
} else if (!matches(*argv, "remote")) {
NEXT_ARG();
daddr = get_addr32(*argv);
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index 84824a5..f631839 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -61,8 +61,8 @@ static int vti6_parse_opt(struct link_util *lu, int argc, char **argv,
struct rtattr *vtiinfo[IFLA_VTI_MAX + 1];
struct in6_addr saddr = IN6ADDR_ANY_INIT;
struct in6_addr daddr = IN6ADDR_ANY_INIT;
- unsigned int ikey = 0;
- unsigned int okey = 0;
+ __be32 ikey = 0;
+ __be32 okey = 0;
unsigned int link = 0;
__u32 fwmark = 0;
int len;
@@ -117,49 +117,14 @@ get_failed:
while (argc > 0) {
if (!matches(*argv, "key")) {
- unsigned int uval;
-
NEXT_ARG();
- if (strchr(*argv, '.'))
- uval = get_addr32(*argv);
- else {
- if (get_unsigned(&uval, *argv, 0) < 0) {
- fprintf(stderr,
- "Invalid value for \"key\": \"%s\"; it should be an unsigned integer\n", *argv);
- exit(-1);
- }
- uval = htonl(uval);
- }
-
- ikey = okey = uval;
+ ikey = okey = tnl_parse_key("key", *argv);
} else if (!matches(*argv, "ikey")) {
- unsigned int uval;
-
NEXT_ARG();
- if (strchr(*argv, '.'))
- uval = get_addr32(*argv);
- else {
- if (get_unsigned(&uval, *argv, 0) < 0) {
- fprintf(stderr, "invalid value for \"ikey\": \"%s\"; it should be an unsigned integer\n", *argv);
- exit(-1);
- }
- uval = htonl(uval);
- }
- ikey = uval;
+ ikey = tnl_parse_key("ikey", *argv);
} else if (!matches(*argv, "okey")) {
- unsigned int uval;
-
NEXT_ARG();
- if (strchr(*argv, '.'))
- uval = get_addr32(*argv);
- else {
- if (get_unsigned(&uval, *argv, 0) < 0) {
- fprintf(stderr, "invalid value for \"okey\": \"%s\"; it should be an unsigned integer\n", *argv);
- exit(-1);
- }
- uval = htonl(uval);
- }
- okey = uval;
+ okey = tnl_parse_key("okey", *argv);
} else if (!matches(*argv, "remote")) {
inet_prefix addr;
diff --git a/ip/tunnel.c b/ip/tunnel.c
index d359eb9..f860103 100644
--- a/ip/tunnel.c
+++ b/ip/tunnel.c
@@ -192,8 +192,9 @@ __be32 tnl_parse_key(const char *name, const char *key)
return get_addr32(key);
if (get_unsigned(&uval, key, 0) < 0) {
- fprintf(stderr, "invalid value for \"%s\": \"%s\";", name, key);
- fprintf(stderr, " it should be an unsigned integer\n");
+ fprintf(stderr,
+ "invalid value for \"%s\": \"%s\"; it should be an unsigned integer\n",
+ name, key);
exit(-1);
}
return htonl(uval);
--
1.7.10.4
^ permalink raw reply related
* [PATCH iproute2 2/3] link_ip6tnl: Use IN6ADDR_ANY_INIT to initialize local/remote endpoints
From: Serhey Popovych @ 2017-12-18 17:48 UTC (permalink / raw)
To: netdev
In-Reply-To: <1513619285-23187-1-git-send-email-serhe.popovych@gmail.com>
Use specialized helper to initialize endpoint addresses with
zeros instead of open coding this. This unifies initialization
style with other ipv6 tunnel variants (i.e. gre6 and vti6).
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/link_ip6tnl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 83a4320..f11ddd5 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -88,8 +88,8 @@ static int ip6tunnel_parse_opt(struct link_util *lu, int argc, char **argv,
struct rtattr *linkinfo[IFLA_INFO_MAX+1];
struct rtattr *iptuninfo[IFLA_IPTUN_MAX + 1];
int len;
- struct in6_addr laddr = {};
- struct in6_addr raddr = {};
+ struct in6_addr laddr = IN6ADDR_ANY_INIT;
+ struct in6_addr raddr = IN6ADDR_ANY_INIT;
__u8 hop_limit = DEFAULT_TNL_HOP_LIMIT;
__u8 encap_limit = IPV6_DEFAULT_TNL_ENCAP_LIMIT;
__u32 flowinfo = 0;
--
1.7.10.4
^ permalink raw reply related
* [PATCH iproute2 3/3] link_vti6: Always add local/remote endpoint attributes
From: Serhey Popovych @ 2017-12-18 17:48 UTC (permalink / raw)
To: netdev
In-Reply-To: <1513619285-23187-1-git-send-email-serhe.popovych@gmail.com>
All tunnels already support for parsing/adding zero
endpoints and vti6 isn't an exception.
This check was added as part of commit 2a80154fde40
(vti6: fix local/remote any addr handling) and looks
too restrictive as purpose of change is to avoid
endpoint configuration from uninitialized data.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/link_vti6.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index f631839..4136b0e 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -154,10 +154,8 @@ get_failed:
addattr32(n, 1024, IFLA_VTI_IKEY, ikey);
addattr32(n, 1024, IFLA_VTI_OKEY, okey);
- if (memcmp(&saddr, &in6addr_any, sizeof(in6addr_any)))
- addattr_l(n, 1024, IFLA_VTI_LOCAL, &saddr, sizeof(saddr));
- if (memcmp(&daddr, &in6addr_any, sizeof(in6addr_any)))
- addattr_l(n, 1024, IFLA_VTI_REMOTE, &daddr, sizeof(daddr));
+ addattr_l(n, 1024, IFLA_VTI_LOCAL, &saddr, sizeof(saddr));
+ addattr_l(n, 1024, IFLA_VTI_REMOTE, &daddr, sizeof(daddr));
addattr32(n, 1024, IFLA_VTI_FWMARK, fwmark);
if (link)
addattr32(n, 1024, IFLA_VTI_LINK, link);
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program
From: Jason Gunthorpe @ 2017-12-18 17:49 UTC (permalink / raw)
To: Knut Omang
Cc: Joe Perches, Stephen Hemminger,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Mauro Carvalho Chehab,
Nicolas Palix, Jonathan Corbet, Santosh Shilimkar, Matthew Wilcox,
cocci-/FJkirnvOdkvYVN+rsErww, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA, Doug Ledford,
Mickaël Salaün, Shuah Khan,
linux-kbuild-u79uwXL29TY76Z2rM5mHXA, Michal Marek, Julia Lawall,
John Haxby, Åsmund Østvold
In-Reply-To: <1513602315.22938.49.camel-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
On Mon, Dec 18, 2017 at 02:05:15PM +0100, Knut Omang wrote:
> https://github.com/torvalds/linux/compare/master...knuto:runchecks
Several of these to rdma/core do not look so big, you should think
about sending them..
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] ss: fix crash with invalid command input file
From: Stephen Hemminger @ 2017-12-18 17:52 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger, Stephen Hemminger
If given an invalid input file with -F flag, ss would crash.
Examples of invalid input are line to long, or null file.
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
misc/ssfilter.y | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/misc/ssfilter.y b/misc/ssfilter.y
index ba82b65f712b..4db3c95faa3c 100644
--- a/misc/ssfilter.y
+++ b/misc/ssfilter.y
@@ -202,15 +202,23 @@ int yylex(void)
argc++;
} else if (yy_fp) {
while (tokptr == NULL) {
- if (fgets(argbuf, sizeof(argbuf)-1, yy_fp) == NULL)
+ size_t len;
+
+ if (fgets(argbuf, sizeof(argbuf), yy_fp) == NULL)
return 0;
- argbuf[sizeof(argbuf)-1] = 0;
- if (strlen(argbuf) == sizeof(argbuf) - 1) {
- fprintf(stderr, "Too long line in filter");
+
+ len = strnlen(argbuf, sizeof(argbuf));
+ if (len == 0) {
+ fprintf(stderr, "Invalid line\n");
+ exit(-1);
+ }
+
+ if (len >= sizeof(argbuf) - 1) {
+ fprintf(stderr, "Too long line in filter\n");
exit(-1);
}
- if (argbuf[strlen(argbuf)-1] == '\n')
- argbuf[strlen(argbuf)-1] = 0;
+ if (argbuf[len - 1] == '\n')
+ argbuf[len-1] = 0;
if (argbuf[0] == '#' || argbuf[0] == '0')
continue;
tokptr = argbuf;
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program
From: Joe Perches @ 2017-12-18 17:53 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Knut Omang, Stephen Hemminger, linux-kernel,
Mauro Carvalho Chehab, Nicolas Palix, Jonathan Corbet,
Santosh Shilimkar, Matthew Wilcox, cocci, rds-devel, linux-rdma,
linux-doc, Doug Ledford, Mickaël Salaün, Shuah Khan,
linux-kbuild, Michal Marek, Julia Lawall, John Haxby,
Åsmund Østvold
In-Reply-To: <20171218174604.GA19056@ziepe.ca>
On Mon, 2017-12-18 at 10:46 -0700, Jason Gunthorpe wrote:
> On Sun, Dec 17, 2017 at 10:00:17PM -0800, Joe Perches wrote:
>
> > > Today when we run checkers we get so many warnings it is too hard to
> > > make any sense of it.
> >
> > Here is a list of the checkpatch messages for drivers/infiniband
> > sorted by type.
> >
> > Many of these might be corrected by using
> >
> > $ ./scripts/checkpatch.pl -f --fix-inplace --types=<TYPE> \
> > $(git ls-files drivers/infiniband/)
>
> How many of these do you think it is worth to fix?
>
> We do get a steady trickle of changes in this topic every cycle.
>
> Is it better to just do a big number of them all at once?
I think so.
> Do you have
> an idea how disruptive this kind of work is to the whole patch flow
> eg new patches no longer applying to for-next, backports no longer
> applying, merge conflicts?
Some do complain about backport patch purity.
I think that difficulty is overstated, but then
again, I don't do backports very often.
I think the best time for any rather wholesale
change is immediately after an rc-1 so overall
in-flight patch conflict volume is minimized.
^ permalink raw reply
* Re: [PATCH bpf-next 12/13] bpf: arm64: add JIT support for multi-function programs
From: Alexei Starovoitov @ 2017-12-18 17:55 UTC (permalink / raw)
To: Daniel Borkmann, Arnd Bergmann, Alexei Starovoitov
Cc: David S . Miller, John Fastabend, Edward Cree, Jakub Kicinski,
Networking, kernel-team
In-Reply-To: <90da62d9-830e-4ea7-b542-ac7a2eb0a811@iogearbox.net>
On 12/18/17 7:51 AM, Daniel Borkmann wrote:
> On 12/18/2017 04:29 PM, Arnd Bergmann wrote:
>> On Fri, Dec 15, 2017 at 2:55 AM, Alexei Starovoitov <ast@kernel.org> wrote:
>>
>>
>>> + if (jit_data->ctx.offset) {
>>> + ctx = jit_data->ctx;
>>> + image_ptr = jit_data->image;
>>> + header = jit_data->header;
>>> + extra_pass = true;
>>> + goto skip_init_ctx;
>>> + }
>>> memset(&ctx, 0, sizeof(ctx));
>>> ctx.prog = prog;
>>
>> The 'goto' jumps over the 'image_size' initialization
>>
>>> prog->bpf_func = (void *)ctx.image;
>>> prog->jited = 1;
>>> prog->jited_len = image_size;
>>
>> so we now get a warning here, starting with linux-next-20171218:
>>
>> arch/arm64/net/bpf_jit_comp.c: In function 'bpf_int_jit_compile':
>> arch/arm64/net/bpf_jit_comp.c:982:18: error: 'image_size' may be used
>> uninitialized in this function [-Werror=maybe-uninitialized]
>>
>> I could not figure out what the code should be doing instead, or if it is
>> indeed safe and the warning is a false-positive.
>
> Good catch, it's buggy indeed. Fix like below is needed; I can submit
> it properly a bit later today:
good catch. My arm gcc 4.8.5 didn't warn about it.
the following would be better fix:
diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 396490cf7316..acaa935ed977 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -897,6 +897,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog
*prog)
image_ptr = jit_data->image;
header = jit_data->header;
extra_pass = true;
+ image_size = sizeof(u32) * ctx.idx;
goto skip_init_ctx;
}
memset(&ctx, 0, sizeof(ctx));
> diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
> index 396490c..a6fd585 100644
> --- a/arch/arm64/net/bpf_jit_comp.c
> +++ b/arch/arm64/net/bpf_jit_comp.c
> @@ -855,6 +855,7 @@ static inline void bpf_flush_icache(void *start, void *end)
> struct arm64_jit_data {
> struct bpf_binary_header *header;
> u8 *image;
> + int image_size;
> struct jit_ctx ctx;
> };
>
> @@ -895,6 +896,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
> if (jit_data->ctx.offset) {
> ctx = jit_data->ctx;
> image_ptr = jit_data->image;
> + image_size = jit_data->image_size;
> header = jit_data->header;
> extra_pass = true;
> goto skip_init_ctx;
> @@ -975,6 +977,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
> } else {
> jit_data->ctx = ctx;
> jit_data->image = image_ptr;
> + jit_data->image_size = image_size;
> jit_data->header = header;
> }
> prog->bpf_func = (void *)ctx.image;
>
^ permalink raw reply related
* Re: [RFC ipsec-next 3/4] net: xfrm: support multiple VTI tunnels
From: David Miller @ 2017-12-18 17:56 UTC (permalink / raw)
To: lorenzo; +Cc: netdev, steffen.klassert, subashab, nharold
In-Reply-To: <20171218161656.40618-4-lorenzo@google.com>
From: Lorenzo Colitti <lorenzo@google.com>
Date: Tue, 19 Dec 2017 01:16:55 +0900
> - ICMP errors are similar to input, except the search is for the
> outbound XFRM state, because the only data that is available is
> the outbound SPI. Thus, ICMP errors are only processed if the
> ikey is the same as the same as the okey. AFAICS this is
> consistent with GRE tunnels, but not with existing VTI
> behaviour.
I think you will need to sort out the VTI ICMP behavior difference
with what exists now.
^ permalink raw reply
* Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program
From: Bart Van Assche @ 2017-12-18 17:56 UTC (permalink / raw)
To: joe@perches.com, jgg@ziepe.ca
Cc: corbet@lwn.net, linux-kernel@vger.kernel.org,
keescook@chromium.org, linux-rdma@vger.kernel.org,
linux-doc@vger.kernel.org, willy@infradead.org,
knut.omang@oracle.com, nicolas.palix@imag.fr,
asmund.ostvold@oracle.com, john.haxby@oracle.com,
alexander.levin@verizon.com, mchehab@kernel.org,
haakon.bugge@oracle.com, michal.lkml@markovi.net,
Gilles.Muller@lip6.fr, "yama
In-Reply-To: <20171218174604.GA19056@ziepe.ca>
On Mon, 2017-12-18 at 10:46 -0700, Jason Gunthorpe wrote:
> On Sun, Dec 17, 2017 at 10:00:17PM -0800, Joe Perches wrote:
>
> > > Today when we run checkers we get so many warnings it is too hard to
> > > make any sense of it.
> >
> > Here is a list of the checkpatch messages for drivers/infiniband
> > sorted by type.
> >
> > Many of these might be corrected by using
> >
> > $ ./scripts/checkpatch.pl -f --fix-inplace --types=<TYPE> \
> > $(git ls-files drivers/infiniband/)
>
> How many of these do you think it is worth to fix?
>
> We do get a steady trickle of changes in this topic every cycle.
>
> Is it better to just do a big number of them all at once? Do you have
> an idea how disruptive this kind of work is to the whole patch flow
> eg new patches no longer applying to for-next, backports no longer
> applying, merge conflicts?
In my opinion patches that only change the coding style and do not change any
functionality are annoying. Before posting a patch that fixes a bug the change
history (git log -p) has to be cheched to figure out which patch introduced
the bug. Patches that only change coding style pollute the change history.
Bart.
^ permalink raw reply
* [PATCH bpf-next] bpf: arm64: fix uninitialized variable
From: Alexei Starovoitov @ 2017-12-18 18:09 UTC (permalink / raw)
To: David S . Miller; +Cc: Daniel Borkmann, Arnd Bergmann, netdev, kernel-team
From: Alexei Starovoitov <ast@fb.com>
fix the following issue:
arch/arm64/net/bpf_jit_comp.c: In function 'bpf_int_jit_compile':
arch/arm64/net/bpf_jit_comp.c:982:18: error: 'image_size' may be used
uninitialized in this function [-Werror=maybe-uninitialized]
Fixes: db496944fdaa ("bpf: arm64: add JIT support for multi-function programs")
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
arch/arm64/net/bpf_jit_comp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 396490cf7316..acaa935ed977 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -897,6 +897,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
image_ptr = jit_data->image;
header = jit_data->header;
extra_pass = true;
+ image_size = sizeof(u32) * ctx.idx;
goto skip_init_ctx;
}
memset(&ctx, 0, sizeof(ctx));
--
2.9.5
^ permalink raw reply related
* [PATCH net-next] bpf/cgroup: fix a verification error for a CGROUP_DEVICE type prog
From: Yonghong Song @ 2017-12-18 18:13 UTC (permalink / raw)
To: ast, daniel, netdev; +Cc: guro, kernel-team
The tools/testing/selftests/bpf test program
test_dev_cgroup fails with the following error
when compiled with llvm 6.0. (I did not try
with earlier versions.)
libbpf: load bpf program failed: Permission denied
libbpf: -- BEGIN DUMP LOG ---
libbpf:
0: (61) r2 = *(u32 *)(r1 +4)
1: (b7) r0 = 0
2: (55) if r2 != 0x1 goto pc+8
R0=inv0 R1=ctx(id=0,off=0,imm=0) R2=inv1 R10=fp0
3: (69) r2 = *(u16 *)(r1 +0)
invalid bpf_context access off=0 size=2
...
The culprit is the following statement in dev_cgroup.c:
short type = ctx->access_type & 0xFFFF;
This code is typical as the ctx->access_type is assigned
as below in kernel/bpf/cgroup.c:
struct bpf_cgroup_dev_ctx ctx = {
.access_type = (access << 16) | dev_type,
.major = major,
.minor = minor,
};
The compiler converts it to u16 access while
the verifier cgroup_dev_is_valid_access rejects
any non u32 access.
This patch permits the field access_type to be accessible
with type u16 and u8 as well.
Signed-off-by: Yonghong Song <yhs@fb.com>
Tested-by: Roman Gushchin <guro@fb.com>
---
include/uapi/linux/bpf.h | 3 ++-
kernel/bpf/cgroup.c | 15 +++++++++++++--
2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 80d62e8..6aa60d4 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1001,7 +1001,8 @@ struct bpf_perf_event_value {
#define BPF_DEVCG_DEV_CHAR (1ULL << 1)
struct bpf_cgroup_dev_ctx {
- __u32 access_type; /* (access << 16) | type */
+ /* access_type encoded as (BPF_DEVCG_ACC_* << 16) | BPF_DEVCG_DEV_* */
+ __u32 access_type;
__u32 major;
__u32 minor;
};
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index b789ab7..c1c0b60 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -568,6 +568,8 @@ static bool cgroup_dev_is_valid_access(int off, int size,
enum bpf_access_type type,
struct bpf_insn_access_aux *info)
{
+ const int size_default = sizeof(__u32);
+
if (type == BPF_WRITE)
return false;
@@ -576,8 +578,17 @@ static bool cgroup_dev_is_valid_access(int off, int size,
/* The verifier guarantees that size > 0. */
if (off % size != 0)
return false;
- if (size != sizeof(__u32))
- return false;
+
+ switch (off) {
+ case bpf_ctx_range(struct bpf_cgroup_dev_ctx, access_type):
+ bpf_ctx_record_field_size(info, size_default);
+ if (!bpf_ctx_narrow_access_ok(off, size, size_default))
+ return false;
+ break;
+ default:
+ if (size != size_default)
+ return false;
+ }
return true;
}
--
2.9.5
^ permalink raw reply related
* Re: [PATCH net-next 0/6] sfc: Initial X2000-series (Medford2) support
From: David Miller @ 2017-12-18 18:16 UTC (permalink / raw)
To: ecree; +Cc: linux-net-drivers, netdev
In-Reply-To: <f9e1279b-03d0-729c-2518-c1e204444447@solarflare.com>
From: Edward Cree <ecree@solarflare.com>
Date: Mon, 18 Dec 2017 16:54:29 +0000
> Basic PCI-level changes to support X2000-series NICs.
> Also fix unexpected-PTP-event log messages, since the timestamp format has
> been changed in these NICs and that causes us to fail to probe PTP (but we
> still get the PPS events).
Series applied.
^ permalink raw reply
* Re: [net 1/1] tipc: fix lost member events bug
From: David Miller @ 2017-12-18 18:17 UTC (permalink / raw)
To: jon.maloy
Cc: netdev, mohan.krishna.ghanta.krishnamurthy, tung.q.nguyen,
hoang.h.le, canh.d.luu, ying.xue, tipc-discussion
In-Reply-To: <1513614856-9857-1-git-send-email-jon.maloy@ericsson.com>
From: Jon Maloy <jon.maloy@ericsson.com>
Date: Mon, 18 Dec 2017 17:34:16 +0100
> Group messages are not supposed to be returned to sender when the
> destination socket disappears. This is done correctly for regular
> traffic messages, by setting the 'dest_droppable' bit in the header.
> But we forget to do that in group protocol messages. This has the effect
> that such messages may sometimes bounce back to the sender, be perceived
> as a legitimate peer message, and wreak general havoc for the rest of
> the session. In particular, we have seen that a member in state LEAVING
> may go back to state RECLAIMED or REMITTED, hence causing suppression
> of an otherwise expected 'member down' event to the user.
>
> We fix this by setting the 'dest_droppable' bit even in group protocol
> messages.
>
> Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
Applied.
^ permalink raw reply
* Re: [net 1/1] tipc: remove leaving group member from all lists
From: David Miller @ 2017-12-18 18:17 UTC (permalink / raw)
To: jon.maloy
Cc: netdev, mohan.krishna.ghanta.krishnamurthy, tung.q.nguyen,
hoang.h.le, canh.d.luu, ying.xue, tipc-discussion
In-Reply-To: <1513617214-15464-1-git-send-email-jon.maloy@ericsson.com>
From: Jon Maloy <jon.maloy@ericsson.com>
Date: Mon, 18 Dec 2017 18:13:34 +0100
> A group member going into state LEAVING should never go back to any
> other state before it is finally deleted. However, this might happen
> if the socket needs to send out a RECLAIM message during this interval.
> Since we forget to remove the leaving member from the group's 'active'
> or 'pending' list, the member might be selected for reclaiming, change
> state to RECLAIMING, and get stuck in this state instead of being
> deleted. This might lead to suppression of the expected 'member down'
> event to the receiver.
>
> We fix this by removing the member from all lists, except the RB tree,
> at the moment it goes into state LEAVING.
>
> Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
Also applied, thanks Jon.
^ 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