* Re: [PATCH 4/4] net: dsa: mv88e6xxx: add PTP support for MV88E6250 family
From: Vladimir Oltean @ 2019-07-30 20:46 UTC (permalink / raw)
To: Richard Cochran
Cc: Hubert Feurstein, netdev, lkml, Andrew Lunn, Vivien Didelot,
Florian Fainelli, David S. Miller, Rasmus Villemoes
In-Reply-To: <20190730171246.GB1251@localhost>
Hi Hubert, Richard,
On Tue, 30 Jul 2019 at 19:44, Richard Cochran <richardcochran@gmail.com> wrote:
>
> On Tue, Jul 30, 2019 at 12:04:29PM +0200, Hubert Feurstein wrote:
> > diff --git a/drivers/net/dsa/mv88e6xxx/ptp.c b/drivers/net/dsa/mv88e6xxx/ptp.c
> > index 768d256f7c9f..51cdf4712517 100644
> > --- a/drivers/net/dsa/mv88e6xxx/ptp.c
> > +++ b/drivers/net/dsa/mv88e6xxx/ptp.c
> > @@ -15,11 +15,38 @@
> > #include "hwtstamp.h"
> > #include "ptp.h"
> >
> > -/* Raw timestamps are in units of 8-ns clock periods. */
> > -#define CC_SHIFT 28
> > -#define CC_MULT (8 << CC_SHIFT)
> > -#define CC_MULT_NUM (1 << 9)
> > -#define CC_MULT_DEM 15625ULL
> > +/* The adjfine API clamps ppb between [-32,768,000, 32,768,000], and
>
> That is not true.
>
I was referring to this:
https://github.com/richardcochran/linuxptp/blob/master/phc.c#L38
/*
* On 32 bit platforms, the PHC driver's maximum adjustment (type
* 'int' in units of ppb) can overflow the timex.freq field (type
* 'long'). So in this case we clamp the maximum to the largest
* possible adjustment that fits into a 32 bit long.
*/
#define BITS_PER_LONG (sizeof(long)*8)
#define MAX_PPB_32 32767999 /* 2^31 - 1 / 65.536 */
Technically it is not "not true".
[snip]
On Tue, 30 Jul 2019 at 21:09, Richard Cochran <richardcochran@gmail.com> wrote:
>
> On Tue, Jul 30, 2019 at 06:20:00PM +0200, Hubert Feurstein wrote:
> > > Please don't re-write this logic. It is written like that for a reason.
> > I used the sja1105_ptp.c as a reference. So it is also wrong there.
>
> I'll let that driver's author worry about that.
>
> Thanks,
> Richard
>
And what is the reason for the neg_adj thing? Can you give an example
of when does the "normal way" of doing signed arithmetics not work?
Thanks,
-Vladimir
^ permalink raw reply
* Re: [PATCH v2 0/3 net-next] Finish conversion of skb_frag_t to bio_vec
From: Jens Axboe @ 2019-07-30 20:49 UTC (permalink / raw)
To: Jonathan Lemon, willy@infradead.org, davem@davemloft.net,
jakub.kicinski@netronome.com
Cc: Kernel Team, netdev@vger.kernel.org
In-Reply-To: <20190730144034.444022-1-jonathan.lemon@gmail.com>
On 7/30/19 8:40 AM, Jonathan Lemon wrote:
> The recent conversion of skb_frag_t to bio_vec did not include
> skb_frag's page_offset. Add accessor functions for this field,
> utilize them, and remove the union, restoring the original structure.
You can add:
Reviewed-by: Jens Axboe <axboe@kernel.dk>
Pretty appalled to see this abomination:
net: Convert skb_frag_t to bio_vec
There are a lot of users of frag->page_offset, so use a union
to avoid converting those users today.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
show up in the net tree without even having been posted on a
block list...
At least this kills this ugly thing.
--
Jens Axboe
^ permalink raw reply
* [PATCH v4 3/3] net/xdp: convert put_page() to put_user_page*()
From: john.hubbard @ 2019-07-30 20:57 UTC (permalink / raw)
To: Andrew Morton
Cc: Al Viro, Christian Benvenuti, Christoph Hellwig, Dan Williams,
Darrick J . Wong, Dave Chinner, Ira Weiny, Jan Kara,
Jason Gunthorpe, Jens Axboe, Jerome Glisse, Kirill A . Shutemov,
Matthew Wilcox, Michal Hocko, Mike Marciniszyn, Mike Rapoport,
linux-block, linux-fsdevel, linux-mm, linux-xfs, LKML,
John Hubbard, Björn Töpel, Magnus Karlsson,
David S . Miller, netdev
In-Reply-To: <20190730205705.9018-1-jhubbard@nvidia.com>
From: John Hubbard <jhubbard@nvidia.com>
For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().
This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").
Cc: Björn Töpel <bjorn.topel@intel.com>
Cc: Magnus Karlsson <magnus.karlsson@intel.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
net/xdp/xdp_umem.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
index 83de74ca729a..17c4b3d3dc34 100644
--- a/net/xdp/xdp_umem.c
+++ b/net/xdp/xdp_umem.c
@@ -166,14 +166,7 @@ void xdp_umem_clear_dev(struct xdp_umem *umem)
static void xdp_umem_unpin_pages(struct xdp_umem *umem)
{
- unsigned int i;
-
- for (i = 0; i < umem->npgs; i++) {
- struct page *page = umem->pgs[i];
-
- set_page_dirty_lock(page);
- put_page(page);
- }
+ put_user_pages_dirty_lock(umem->pgs, umem->npgs, true);
kfree(umem->pgs);
umem->pgs = NULL;
--
2.22.0
^ permalink raw reply related
* Re: [PATCH bpf-next] tools: bpftool: add support for reporting the effective cgroup progs
From: Jakub Kicinski @ 2019-07-30 21:00 UTC (permalink / raw)
To: Takshak Chahande
Cc: alexei.starovoitov@gmail.com, daniel@iogearbox.net,
netdev@vger.kernel.org, bpf@vger.kernel.org,
oss-drivers@netronome.com, Kernel Team, Quentin Monnet
In-Reply-To: <20190730180443.GA48276@ctakshak-mbp.dhcp.thefacebook.com>
On Tue, 30 Jul 2019 18:04:53 +0000, Takshak Chahande wrote:
> Jakub Kicinski <jakub.kicinski@netronome.com> wrote on Mon [2019-Jul-29 14:35:38 -0700]:
> > @@ -158,20 +161,30 @@ static int show_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type,
> > static int do_show(int argc, char **argv)
> > {
> > enum bpf_attach_type type;
> > + const char *path;
> > int cgroup_fd;
> > int ret = -1;
> >
> > - if (argc < 1) {
> > - p_err("too few parameters for cgroup show");
> > - goto exit;
> > - } else if (argc > 1) {
> > - p_err("too many parameters for cgroup show");
> > - goto exit;
> > + query_flags = 0;
> > +
> > + if (!REQ_ARGS(1))
> > + return -1;
> > + path = GET_ARG();
> > +
> > + while (argc) {
> > + if (is_prefix(*argv, "effective")) {
> > + query_flags |= BPF_F_QUERY_EFFECTIVE;
> > + NEXT_ARG();
> > + } else {
> > + p_err("expected no more arguments, 'effective', got: '%s'?",
> > + *argv);
> > + return -1;
> > + }
> > }
> This while loop will allow multiple 'effective' keywords in the argument
> unnecessarily. IMO, we should strictly restrict only for single
> occurance of 'effective' word.
It's kind of the way all bpftool works to date :(
But perhaps not checking is worse than inconsistency? Okay, let's fix
this up.
> > - cgroup_fd = open(argv[0], O_RDONLY);
> > + cgroup_fd = open(path, O_RDONLY);
> > if (cgroup_fd < 0) {
> > - p_err("can't open cgroup %s", argv[0]);
> > + p_err("can't open cgroup %s", path);
> > goto exit;
> > }
> >
> > @@ -297,23 +310,29 @@ static int do_show_tree(int argc, char **argv)
> > char *cgroup_root;
> > int ret;
> >
> > - switch (argc) {
> > - case 0:
> > + query_flags = 0;
> > +
> > + if (!argc) {
> > cgroup_root = find_cgroup_root();
> > if (!cgroup_root) {
> > p_err("cgroup v2 isn't mounted");
> > return -1;
> > }
> > - break;
> > - case 1:
> > - cgroup_root = argv[0];
> > - break;
> > - default:
> > - p_err("too many parameters for cgroup tree");
> > - return -1;
> > + } else {
> > + cgroup_root = GET_ARG();
> > +
> > + while (argc) {
> > + if (is_prefix(*argv, "effective")) {
> > + query_flags |= BPF_F_QUERY_EFFECTIVE;
> > + NEXT_ARG();
>
> NEXT_ARG() does update argc value; that means after this outer if/else we need
> to know how argc has become 0 (through which path) before freeing up `cgroup_root` allocated
> memory later at the end of this function.
Good catch!
> > + } else {
> > + p_err("expected no more arguments, 'effective', got: '%s'?",
> > + *argv);
> > + return -1;
> > + }
> > + }
> > }
> Thanks for the patch. Apart from above two issues, patch looks good.
Thanks for the review.
^ permalink raw reply
* Re: [patch net-next 3/3] netdevsim: create devlink and netdev instances in namespace
From: Jiri Pirko @ 2019-07-30 21:03 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, davem, sthemmin, dsahern, mlxsw
In-Reply-To: <20190730101411.7dc1e83d@cakuba.netronome.com>
Tue, Jul 30, 2019 at 07:14:11PM CEST, jakub.kicinski@netronome.com wrote:
>On Tue, 30 Jul 2019 08:06:55 +0200, Jiri Pirko wrote:
>> >> diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
>> >> index 79c05af2a7c0..cdf53d0e0c49 100644
>> >> --- a/drivers/net/netdevsim/netdevsim.h
>> >> +++ b/drivers/net/netdevsim/netdevsim.h
>> >> @@ -19,6 +19,7 @@
>> >> #include <linux/netdevice.h>
>> >> #include <linux/u64_stats_sync.h>
>> >> #include <net/devlink.h>
>> >> +#include <net/net_namespace.h>
>> >
>> >You can just do a forward declaration, no need to pull in the header.
>>
>> Sure, but why?
>
>Less time to compile the kernel after net_namespace.h was touched.
>Don't we all spend more time that we would like to recompiling the
>kernel? :( Not a huge deal if you have a strong preference.
I removed it in v2. I don't care that much either :)
^ permalink raw reply
* [PATCH bpf-next v2] tools: bpftool: add support for reporting the effective cgroup progs
From: Jakub Kicinski @ 2019-07-30 21:03 UTC (permalink / raw)
To: alexei.starovoitov, daniel
Cc: netdev, bpf, oss-drivers, ctakshak, kernel-team, Jakub Kicinski,
Quentin Monnet
Takshak said in the original submission:
With different bpf attach_flags available to attach bpf programs specially
with BPF_F_ALLOW_OVERRIDE and BPF_F_ALLOW_MULTI, the list of effective
bpf-programs available to any sub-cgroups really needs to be available for
easy debugging.
Using BPF_F_QUERY_EFFECTIVE flag, one can get the list of not only attached
bpf-programs to a cgroup but also the inherited ones from parent cgroup.
So a new option is introduced to use BPF_F_QUERY_EFFECTIVE query flag here
to list all the effective bpf-programs available for execution at a specified
cgroup.
Reused modified test program test_cgroup_attach from tools/testing/selftests/bpf:
# ./test_cgroup_attach
With old bpftool:
# bpftool cgroup show /sys/fs/cgroup/cgroup-test-work-dir/cg1/
ID AttachType AttachFlags Name
271 egress multi pkt_cntr_1
272 egress multi pkt_cntr_2
Attached new program pkt_cntr_4 in cg2 gives following:
# bpftool cgroup show /sys/fs/cgroup/cgroup-test-work-dir/cg1/cg2
ID AttachType AttachFlags Name
273 egress override pkt_cntr_4
And with new "effective" option it shows all effective programs for cg2:
# bpftool cgroup show /sys/fs/cgroup/cgroup-test-work-dir/cg1/cg2 effective
ID AttachType AttachFlags Name
273 egress override pkt_cntr_4
271 egress override pkt_cntr_1
272 egress override pkt_cntr_2
Compared to original submission use a local flag instead of global
option.
We need to clear query_flags on every command, in case batch mode
wants to use varying settings.
v2: (Takshak)
- forbid duplicated flags;
- fix cgroup path freeing.
Signed-off-by: Takshak Chahande <ctakshak@fb.com>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
.../bpftool/Documentation/bpftool-cgroup.rst | 16 +++-
tools/bpf/bpftool/bash-completion/bpftool | 15 ++--
tools/bpf/bpftool/cgroup.c | 83 ++++++++++++-------
3 files changed, 76 insertions(+), 38 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
index 585f270c2d25..06a28b07787d 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
@@ -20,8 +20,8 @@ SYNOPSIS
CGROUP COMMANDS
===============
-| **bpftool** **cgroup { show | list }** *CGROUP*
-| **bpftool** **cgroup tree** [*CGROUP_ROOT*]
+| **bpftool** **cgroup { show | list }** *CGROUP* [**effective**]
+| **bpftool** **cgroup tree** [*CGROUP_ROOT*] [**effective**]
| **bpftool** **cgroup attach** *CGROUP* *ATTACH_TYPE* *PROG* [*ATTACH_FLAGS*]
| **bpftool** **cgroup detach** *CGROUP* *ATTACH_TYPE* *PROG*
| **bpftool** **cgroup help**
@@ -35,13 +35,17 @@ CGROUP COMMANDS
DESCRIPTION
===========
- **bpftool cgroup { show | list }** *CGROUP*
+ **bpftool cgroup { show | list }** *CGROUP* [**effective**]
List all programs attached to the cgroup *CGROUP*.
Output will start with program ID followed by attach type,
attach flags and program name.
- **bpftool cgroup tree** [*CGROUP_ROOT*]
+ If **effective** is specified retrieve effective programs that
+ will execute for events within a cgroup. This includes
+ inherited along with attached ones.
+
+ **bpftool cgroup tree** [*CGROUP_ROOT*] [**effective**]
Iterate over all cgroups in *CGROUP_ROOT* and list all
attached programs. If *CGROUP_ROOT* is not specified,
bpftool uses cgroup v2 mountpoint.
@@ -50,6 +54,10 @@ DESCRIPTION
commands: it starts with absolute cgroup path, followed by
program ID, attach type, attach flags and program name.
+ If **effective** is specified retrieve effective programs that
+ will execute for events within a cgroup. This includes
+ inherited along with attached ones.
+
**bpftool cgroup attach** *CGROUP* *ATTACH_TYPE* *PROG* [*ATTACH_FLAGS*]
Attach program *PROG* to the cgroup *CGROUP* with attach type
*ATTACH_TYPE* and optional *ATTACH_FLAGS*.
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 6b961a5ed100..df16c5415444 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -710,12 +710,15 @@ _bpftool()
;;
cgroup)
case $command in
- show|list)
- _filedir
- return 0
- ;;
- tree)
- _filedir
+ show|list|tree)
+ case $cword in
+ 3)
+ _filedir
+ ;;
+ 4)
+ COMPREPLY=( $( compgen -W 'effective' -- "$cur" ) )
+ ;;
+ esac
return 0
;;
attach|detach)
diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
index f3c05b08c68c..44352b5aca85 100644
--- a/tools/bpf/bpftool/cgroup.c
+++ b/tools/bpf/bpftool/cgroup.c
@@ -29,6 +29,8 @@
" recvmsg4 | recvmsg6 | sysctl |\n" \
" getsockopt | setsockopt }"
+static unsigned int query_flags;
+
static const char * const attach_type_strings[] = {
[BPF_CGROUP_INET_INGRESS] = "ingress",
[BPF_CGROUP_INET_EGRESS] = "egress",
@@ -107,7 +109,8 @@ static int count_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type)
__u32 prog_cnt = 0;
int ret;
- ret = bpf_prog_query(cgroup_fd, type, 0, NULL, NULL, &prog_cnt);
+ ret = bpf_prog_query(cgroup_fd, type, query_flags, NULL,
+ NULL, &prog_cnt);
if (ret)
return -1;
@@ -125,8 +128,8 @@ static int show_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type,
int ret;
prog_cnt = ARRAY_SIZE(prog_ids);
- ret = bpf_prog_query(cgroup_fd, type, 0, &attach_flags, prog_ids,
- &prog_cnt);
+ ret = bpf_prog_query(cgroup_fd, type, query_flags, &attach_flags,
+ prog_ids, &prog_cnt);
if (ret)
return ret;
@@ -158,20 +161,34 @@ static int show_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type,
static int do_show(int argc, char **argv)
{
enum bpf_attach_type type;
+ const char *path;
int cgroup_fd;
int ret = -1;
- if (argc < 1) {
- p_err("too few parameters for cgroup show");
- goto exit;
- } else if (argc > 1) {
- p_err("too many parameters for cgroup show");
- goto exit;
+ query_flags = 0;
+
+ if (!REQ_ARGS(1))
+ return -1;
+ path = GET_ARG();
+
+ while (argc) {
+ if (is_prefix(*argv, "effective")) {
+ if (query_flags & BPF_F_QUERY_EFFECTIVE) {
+ p_err("duplicated argument: %s", *argv);
+ return -1;
+ }
+ query_flags |= BPF_F_QUERY_EFFECTIVE;
+ NEXT_ARG();
+ } else {
+ p_err("expected no more arguments, 'effective', got: '%s'?",
+ *argv);
+ return -1;
+ }
}
- cgroup_fd = open(argv[0], O_RDONLY);
+ cgroup_fd = open(path, O_RDONLY);
if (cgroup_fd < 0) {
- p_err("can't open cgroup %s", argv[0]);
+ p_err("can't open cgroup %s", path);
goto exit;
}
@@ -294,26 +311,37 @@ static char *find_cgroup_root(void)
static int do_show_tree(int argc, char **argv)
{
- char *cgroup_root;
+ char *cgroup_root, *cgroup_alloced = NULL;
int ret;
- switch (argc) {
- case 0:
- cgroup_root = find_cgroup_root();
- if (!cgroup_root) {
+ query_flags = 0;
+
+ if (!argc) {
+ cgroup_alloced = find_cgroup_root();
+ if (!cgroup_alloced) {
p_err("cgroup v2 isn't mounted");
return -1;
}
- break;
- case 1:
- cgroup_root = argv[0];
- break;
- default:
- p_err("too many parameters for cgroup tree");
- return -1;
+ cgroup_root = cgroup_alloced;
+ } else {
+ cgroup_root = GET_ARG();
+
+ while (argc) {
+ if (is_prefix(*argv, "effective")) {
+ if (query_flags & BPF_F_QUERY_EFFECTIVE) {
+ p_err("duplicated argument: %s", *argv);
+ return -1;
+ }
+ query_flags |= BPF_F_QUERY_EFFECTIVE;
+ NEXT_ARG();
+ } else {
+ p_err("expected no more arguments, 'effective', got: '%s'?",
+ *argv);
+ return -1;
+ }
+ }
}
-
if (json_output)
jsonw_start_array(json_wtr);
else
@@ -338,8 +366,7 @@ static int do_show_tree(int argc, char **argv)
if (json_output)
jsonw_end_array(json_wtr);
- if (argc == 0)
- free(cgroup_root);
+ free(cgroup_alloced);
return ret;
}
@@ -459,8 +486,8 @@ static int do_help(int argc, char **argv)
}
fprintf(stderr,
- "Usage: %s %s { show | list } CGROUP\n"
- " %s %s tree [CGROUP_ROOT]\n"
+ "Usage: %s %s { show | list } CGROUP [**effective**]\n"
+ " %s %s tree [CGROUP_ROOT] [**effective**]\n"
" %s %s attach CGROUP ATTACH_TYPE PROG [ATTACH_FLAGS]\n"
" %s %s detach CGROUP ATTACH_TYPE PROG\n"
" %s %s help\n"
--
2.21.0
^ permalink raw reply related
* Re: [PATCH] net: usb: pegasus: fix improper read if get_registers() fail
From: David Miller @ 2019-07-30 21:10 UTC (permalink / raw)
To: kirjanov; +Cc: kda, petkan, netdev
In-Reply-To: <CAHj3AVm2EZB7n9UBxiBmA+6XN+EgAC_FRoHjh6kO3WMT8KVd6g@mail.gmail.com>
From: Denis Kirjanov <kirjanov@gmail.com>
Date: Tue, 30 Jul 2019 21:19:46 +0300
> On Tuesday, July 30, 2019, David Miller <davem@davemloft.net> wrote:
>
>> From: Denis Kirjanov <kda@linux-powerpc.org>
>> Date: Tue, 30 Jul 2019 15:13:57 +0200
>>
>> > get_registers() may fail with -ENOMEM and in this
>> > case we can read a garbage from the status variable tmp.
>> >
>> > Reported-by: syzbot+3499a83b2d062ae409d4@syzkaller.appspotmail.com
>> > Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
>>
>> Why did you post this patch twice? What is different between the two
>> versions?
>
>
> Looks like it’s the issue with git send-email :/
> Do you want me to figure out the reason and resend?
No need, I was just curious.
^ permalink raw reply
* [PATCH net-next] net/tls: prevent skb_orphan() from leaking TLS plain text with offload
From: Jakub Kicinski @ 2019-07-30 21:12 UTC (permalink / raw)
To: davem
Cc: netdev, oss-drivers, edumazet, borisp, aviadye, davejwatson,
john.fastabend, daniel, willemb, xiyou.wangcong, fw,
alexei.starovoitov, Jakub Kicinski
sk_validate_xmit_skb() and drivers depend on the sk member of
struct sk_buff to identify segments requiring encryption.
Any operation which removes or does not preserve the original TLS
socket such as skb_orphan() or skb_clone() will cause clear text
leaks.
Make the TCP socket underlying an offloaded TLS connection
mark all skbs as decrypted, if TLS TX is in offload mode.
Then in sk_validate_xmit_skb() catch skbs which have no socket
(or a socket with no validation) and decrypted flag set.
Note that CONFIG_SOCK_VALIDATE_XMIT, CONFIG_TLS_DEVICE and
sk->sk_validate_xmit_skb are slightly interchangeable right now,
they all imply TLS offload. The new checks are guarded by
CONFIG_TLS_DEVICE because that's the option guarding the
sk_buff->decrypted member.
Second, smaller issue with orphaning is that it breaks
the guarantee that packets will be delivered to device
queues in-order. All TLS offload drivers depend on that
scheduling property. This means skb_orphan_partial()'s
trick of preserving partial socket references will cause
issues in the drivers. We need a full orphan, and as a
result netem delay/throttling will cause all TLS offload
skbs to be dropped.
Reusing the sk_buff->decrypted flag also protects from
leaking clear text when incoming, decrypted skb is redirected
(e.g. by TC).
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
I'm sending this for net-next because of lack of confidence
in my own abilities. It should apply cleanly to net... :)
Documentation/networking/tls-offload.rst | 9 --------
include/net/sock.h | 28 +++++++++++++++++++++++-
net/core/skbuff.c | 3 +++
net/core/sock.c | 22 ++++++++++++++-----
net/ipv4/tcp.c | 4 +++-
net/ipv4/tcp_output.c | 3 +++
net/tls/tls_device.c | 2 ++
7 files changed, 55 insertions(+), 16 deletions(-)
diff --git a/Documentation/networking/tls-offload.rst b/Documentation/networking/tls-offload.rst
index 048e5ca44824..2bc3ab5515d8 100644
--- a/Documentation/networking/tls-offload.rst
+++ b/Documentation/networking/tls-offload.rst
@@ -499,15 +499,6 @@ offloads, old connections will remain active after flags are cleared.
Known bugs
==========
-skb_orphan() leaks clear text
------------------------------
-
-Currently drivers depend on the :c:member:`sk` member of
-:c:type:`struct sk_buff <sk_buff>` to identify segments requiring
-encryption. Any operation which removes or does not preserve the socket
-association such as :c:func:`skb_orphan` or :c:func:`skb_clone`
-will cause the driver to miss the packets and lead to clear text leaks.
-
Redirects leak clear text
-------------------------
diff --git a/include/net/sock.h b/include/net/sock.h
index 228db3998e46..90f3f552c789 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -814,6 +814,7 @@ enum sock_flags {
SOCK_TXTIME,
SOCK_XDP, /* XDP is attached */
SOCK_TSTAMP_NEW, /* Indicates 64 bit timestamps always */
+ SOCK_CRYPTO_TX_PLAIN_TEXT, /* Generate skbs with decrypted flag set */
};
#define SK_FLAGS_TIMESTAMP ((1UL << SOCK_TIMESTAMP) | (1UL << SOCK_TIMESTAMPING_RX_SOFTWARE))
@@ -2480,8 +2481,26 @@ static inline bool sk_fullsock(const struct sock *sk)
return (1 << sk->sk_state) & ~(TCPF_TIME_WAIT | TCPF_NEW_SYN_RECV);
}
+static inline void sk_set_tx_crypto(const struct sock *sk, struct sk_buff *skb)
+{
+#ifdef CONFIG_TLS_DEVICE
+ skb->decrypted = sock_flag(sk, SOCK_CRYPTO_TX_PLAIN_TEXT);
+#endif
+}
+
+static inline bool sk_tx_crypto_match(const struct sock *sk,
+ const struct sk_buff *skb)
+{
+#ifdef CONFIG_TLS_DEVICE
+ return sock_flag(sk, SOCK_CRYPTO_TX_PLAIN_TEXT) == !!skb->decrypted;
+#else
+ return true;
+#endif
+}
+
/* Checks if this SKB belongs to an HW offloaded socket
* and whether any SW fallbacks are required based on dev.
+ * Check decrypted mark in case skb_orphan() cleared socket.
*/
static inline struct sk_buff *sk_validate_xmit_skb(struct sk_buff *skb,
struct net_device *dev)
@@ -2489,8 +2508,15 @@ static inline struct sk_buff *sk_validate_xmit_skb(struct sk_buff *skb,
#ifdef CONFIG_SOCK_VALIDATE_XMIT
struct sock *sk = skb->sk;
- if (sk && sk_fullsock(sk) && sk->sk_validate_xmit_skb)
+ if (sk && sk_fullsock(sk) && sk->sk_validate_xmit_skb) {
skb = sk->sk_validate_xmit_skb(sk, dev, skb);
+#ifdef CONFIG_TLS_DEVICE
+ } else if (unlikely(skb->decrypted)) {
+ pr_warn_ratelimited("unencrypted skb with no associated socket - dropping\n");
+ kfree_skb(skb);
+ skb = NULL;
+#endif
+ }
#endif
return skb;
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 0b788df5a75b..9e92684479b9 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3794,6 +3794,9 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
skb_reserve(nskb, headroom);
__skb_put(nskb, doffset);
+#ifdef CONFIG_TLS_DEVICE
+ nskb->decrypted = head_skb->decrypted;
+#endif
}
if (segs)
diff --git a/net/core/sock.c b/net/core/sock.c
index d57b0cc995a0..b0c10b518e65 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1992,6 +1992,22 @@ void skb_set_owner_w(struct sk_buff *skb, struct sock *sk)
}
EXPORT_SYMBOL(skb_set_owner_w);
+static bool can_skb_orphan_partial(const struct sk_buff *skb)
+{
+#ifdef CONFIG_TLS_DEVICE
+ /* Drivers depend on in-order delivery for crypto offload,
+ * partial orphan breaks out-of-order-OK logic.
+ */
+ if (skb->decrypted)
+ return false;
+#endif
+#ifdef CONFIG_INET
+ if (skb->destructor == tcp_wfree)
+ return true;
+#endif
+ return skb->destructor == sock_wfree;
+}
+
/* This helper is used by netem, as it can hold packets in its
* delay queue. We want to allow the owner socket to send more
* packets, as if they were already TX completed by a typical driver.
@@ -2003,11 +2019,7 @@ void skb_orphan_partial(struct sk_buff *skb)
if (skb_is_tcp_pure_ack(skb))
return;
- if (skb->destructor == sock_wfree
-#ifdef CONFIG_INET
- || skb->destructor == tcp_wfree
-#endif
- ) {
+ if (can_skb_orphan_partial(skb)) {
struct sock *sk = skb->sk;
if (refcount_inc_not_zero(&sk->sk_refcnt)) {
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index f62f0e7e3cdd..dee93eab02fe 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -984,6 +984,7 @@ ssize_t do_tcp_sendpages(struct sock *sk, struct page *page, int offset,
if (!skb)
goto wait_for_memory;
+ sk_set_tx_crypto(sk, skb);
skb_entail(sk, skb);
copy = size_goal;
}
@@ -993,7 +994,8 @@ ssize_t do_tcp_sendpages(struct sock *sk, struct page *page, int offset,
i = skb_shinfo(skb)->nr_frags;
can_coalesce = skb_can_coalesce(skb, i, page, offset);
- if (!can_coalesce && i >= sysctl_max_skb_frags) {
+ if ((!can_coalesce && i >= sysctl_max_skb_frags) ||
+ !sk_tx_crypto_match(sk, skb)) {
tcp_mark_push(tp, skb);
goto new_segment;
}
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 6e4afc48d7bb..9efd0ca44d49 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1320,6 +1320,7 @@ int tcp_fragment(struct sock *sk, enum tcp_queue tcp_queue,
buff = sk_stream_alloc_skb(sk, nsize, gfp, true);
if (!buff)
return -ENOMEM; /* We'll just try again later. */
+ sk_set_tx_crypto(sk, buff);
sk->sk_wmem_queued += buff->truesize;
sk_mem_charge(sk, buff->truesize);
@@ -1874,6 +1875,7 @@ static int tso_fragment(struct sock *sk, struct sk_buff *skb, unsigned int len,
buff = sk_stream_alloc_skb(sk, 0, gfp, true);
if (unlikely(!buff))
return -ENOMEM;
+ sk_set_tx_crypto(sk, buff);
sk->sk_wmem_queued += buff->truesize;
sk_mem_charge(sk, buff->truesize);
@@ -2139,6 +2141,7 @@ static int tcp_mtu_probe(struct sock *sk)
nskb = sk_stream_alloc_skb(sk, probe_size, GFP_ATOMIC, false);
if (!nskb)
return -1;
+ sk_set_tx_crypto(sk, nskb);
sk->sk_wmem_queued += nskb->truesize;
sk_mem_charge(sk, nskb->truesize);
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index 4ec8a06fa5d1..3d78742b3b1b 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -970,6 +970,8 @@ int tls_set_device_offload(struct sock *sk, struct tls_context *ctx)
tls_device_attach(ctx, sk, netdev);
+ sock_set_flag(sk, SOCK_CRYPTO_TX_PLAIN_TEXT);
+
/* following this assignment tls_is_sk_tx_device_offloaded
* will return true and the context might be accessed
* by the netdev's xmit function.
--
2.21.0
^ permalink raw reply related
* Re: [PATCHv2 net-next 0/5] sctp: clean up __sctp_connect function
From: David Miller @ 2019-07-30 21:18 UTC (permalink / raw)
To: marcelo.leitner; +Cc: lucien.xin, netdev, linux-sctp, nhorman
In-Reply-To: <20190730194231.GE4064@localhost.localdomain>
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: Tue, 30 Jul 2019 16:42:31 -0300
> On Tue, Jul 30, 2019 at 08:38:18PM +0800, Xin Long wrote:
>> This patchset is to factor out some common code for
>> sctp_sendmsg_new_asoc() and __sctp_connect() into 2
>> new functioins.
>>
>> v1->v2:
>> - add the patch 1/5 to avoid a slab-out-of-bounds warning.
>> - add some code comment for the check change in patch 2/5.
>> - remove unused 'addrcnt' as Marcelo noticed in patch 3/5.
>>
>> Xin Long (5):
>> sctp: only copy the available addr data in sctp_transport_init
>> sctp: check addr_size with sa_family_t size in
>> __sctp_setsockopt_connectx
>> sctp: clean up __sctp_connect
>> sctp: factor out sctp_connect_new_asoc
>> sctp: factor out sctp_connect_add_peer
>
> Series,
> Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH v2 bpf-next 01/12] libbpf: add .BTF.ext offset relocation section loading
From: Song Liu @ 2019-07-30 21:19 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
Yonghong Song, Andrii Nakryiko, Kernel Team
In-Reply-To: <20190730195408.670063-2-andriin@fb.com>
> On Jul 30, 2019, at 12:53 PM, Andrii Nakryiko <andriin@fb.com> wrote:
>
> Add support for BPF CO-RE offset relocations. Add section/record
> iteration macros for .BTF.ext. These macro are useful for iterating over
> each .BTF.ext record, either for dumping out contents or later for BPF
> CO-RE relocation handling.
>
> To enable other parts of libbpf to work with .BTF.ext contents, moved
> a bunch of type definitions into libbpf_internal.h.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>
^ permalink raw reply
* Re: [PATCH v2 0/3 net-next] Finish conversion of skb_frag_t to bio_vec
From: David Miller @ 2019-07-30 21:22 UTC (permalink / raw)
To: jonathan.lemon; +Cc: willy, jakub.kicinski, kernel-team, netdev
In-Reply-To: <20190730144034.444022-1-jonathan.lemon@gmail.com>
From: Jonathan Lemon <jonathan.lemon@gmail.com>
Date: Tue, 30 Jul 2019 07:40:31 -0700
> The recent conversion of skb_frag_t to bio_vec did not include
> skb_frag's page_offset. Add accessor functions for this field,
> utilize them, and remove the union, restoring the original structure.
>
> v2:
> - rename accessors
> - follow kdoc conventions
Series applied, thanks Jonathan.
^ permalink raw reply
* Re: [PATCH v2 0/3 net-next] Finish conversion of skb_frag_t to bio_vec
From: David Miller @ 2019-07-30 21:22 UTC (permalink / raw)
To: axboe; +Cc: jonathan.lemon, willy, jakub.kicinski, Kernel-team, netdev
In-Reply-To: <1d34658b-a807-44ae-756a-d55dead27f94@fb.com>
From: Jens Axboe <axboe@fb.com>
Date: Tue, 30 Jul 2019 20:49:09 +0000
> Pretty appalled to see this abomination:
>
> net: Convert skb_frag_t to bio_vec
>
> There are a lot of users of frag->page_offset, so use a union
> to avoid converting those users today.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
> show up in the net tree without even having been posted on a
> block list...
>
> At least this kills this ugly thing.
Sorry about that Jens, but at least as you say it's gone now.
^ permalink raw reply
* Re: [PATCH v2 bpf-next 03/12] selftests/bpf: add BPF_CORE_READ relocatable read macro
From: Song Liu @ 2019-07-30 21:24 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Networking, Alexei Starovoitov, daniel@iogearbox.net,
Yonghong Song, andrii.nakryiko@gmail.com, Kernel Team
In-Reply-To: <20190730195408.670063-4-andriin@fb.com>
> On Jul 30, 2019, at 12:53 PM, Andrii Nakryiko <andriin@fb.com> wrote:
>
> Add BPF_CORE_READ macro used in tests to do bpf_core_read(), which
> automatically captures offset relocation.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
> tools/testing/selftests/bpf/bpf_helpers.h | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
> index f804f210244e..81bc51293d11 100644
> --- a/tools/testing/selftests/bpf/bpf_helpers.h
> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
> @@ -501,4 +501,23 @@ struct pt_regs;
> (void *)(PT_REGS_FP(ctx) + sizeof(ip))); })
> #endif
>
> +/*
> + * BPF_CORE_READ abstracts away bpf_probe_read() call and captures offset
> + * relocation for source address using __builtin_preserve_access_index()
> + * built-in, provided by Clang.
> + *
> + * __builtin_preserve_access_index() takes as an argument an expression of
> + * taking an address of a field within struct/union. It makes compiler emit
> + * a relocation, which records BTF type ID describing root struct/union and an
> + * accessor string which describes exact embedded field that was used to take
> + * an address. See detailed description of this relocation format and
> + * semantics in comments to struct bpf_offset_reloc in libbpf_internal.h.
> + *
> + * This relocation allows libbpf to adjust BPF instruction to use correct
> + * actual field offset, based on target kernel BTF type that matches original
> + * (local) BTF, used to record relocation.
> + */
> +#define BPF_CORE_READ(dst, src) \
> + bpf_probe_read(dst, sizeof(*src), __builtin_preserve_access_index(src))
We should use "sizeof(*(src))"
^ permalink raw reply
* Re: [PATCH v6 51/57] net: Remove dev_err() usage after platform_get_irq()
From: David Miller @ 2019-07-30 21:25 UTC (permalink / raw)
To: swboyd
Cc: linux-kernel, kvalo, saeedm, jeffrey.t.kirsher, nbd, lorenzo,
netdev, gregkh
In-Reply-To: <20190730181557.90391-52-swboyd@chromium.org>
From: Stephen Boyd <swboyd@chromium.org>
Date: Tue, 30 Jul 2019 11:15:51 -0700
> We don't need dev_err() messages when platform_get_irq() fails now that
> platform_get_irq() prints an error message itself when something goes
> wrong. Let's remove these prints with a simple semantic patch.
...
> While we're here, remove braces on if statements that only have one
> statement (manually).
...
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>
> Please apply directly to subsystem trees
I'll take this into net-next.
^ permalink raw reply
* Re: [PATCH v2 bpf-next 03/12] selftests/bpf: add BPF_CORE_READ relocatable read macro
From: Andrii Nakryiko @ 2019-07-30 21:26 UTC (permalink / raw)
To: Song Liu
Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
daniel@iogearbox.net, Yonghong Song, Kernel Team
In-Reply-To: <87422673-525B-461B-B487-EB16386CAB25@fb.com>
On Tue, Jul 30, 2019 at 2:24 PM Song Liu <songliubraving@fb.com> wrote:
>
>
>
> > On Jul 30, 2019, at 12:53 PM, Andrii Nakryiko <andriin@fb.com> wrote:
> >
> > Add BPF_CORE_READ macro used in tests to do bpf_core_read(), which
> > automatically captures offset relocation.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> > tools/testing/selftests/bpf/bpf_helpers.h | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
> > index f804f210244e..81bc51293d11 100644
> > --- a/tools/testing/selftests/bpf/bpf_helpers.h
> > +++ b/tools/testing/selftests/bpf/bpf_helpers.h
> > @@ -501,4 +501,23 @@ struct pt_regs;
> > (void *)(PT_REGS_FP(ctx) + sizeof(ip))); })
> > #endif
> >
> > +/*
> > + * BPF_CORE_READ abstracts away bpf_probe_read() call and captures offset
> > + * relocation for source address using __builtin_preserve_access_index()
> > + * built-in, provided by Clang.
> > + *
> > + * __builtin_preserve_access_index() takes as an argument an expression of
> > + * taking an address of a field within struct/union. It makes compiler emit
> > + * a relocation, which records BTF type ID describing root struct/union and an
> > + * accessor string which describes exact embedded field that was used to take
> > + * an address. See detailed description of this relocation format and
> > + * semantics in comments to struct bpf_offset_reloc in libbpf_internal.h.
> > + *
> > + * This relocation allows libbpf to adjust BPF instruction to use correct
> > + * actual field offset, based on target kernel BTF type that matches original
> > + * (local) BTF, used to record relocation.
> > + */
> > +#define BPF_CORE_READ(dst, src) \
> > + bpf_probe_read(dst, sizeof(*src), __builtin_preserve_access_index(src))
>
> We should use "sizeof(*(src))"
>
Good point. Also (dst) instead of just (dst). Will update.
^ permalink raw reply
* Re: [PATCH v2 0/3 net-next] Finish conversion of skb_frag_t to bio_vec
From: Jens Axboe @ 2019-07-30 21:30 UTC (permalink / raw)
To: David Miller
Cc: jonathan.lemon@gmail.com, willy@infradead.org,
jakub.kicinski@netronome.com, Kernel Team, netdev@vger.kernel.org
In-Reply-To: <20190730.142238.1475873068715429404.davem@davemloft.net>
On 7/30/19 3:22 PM, David Miller wrote:
> From: Jens Axboe <axboe@fb.com>
> Date: Tue, 30 Jul 2019 20:49:09 +0000
>
>> Pretty appalled to see this abomination:
>>
>> net: Convert skb_frag_t to bio_vec
>>
>> There are a lot of users of frag->page_offset, so use a union
>> to avoid converting those users today.
>>
>> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
>> Signed-off-by: David S. Miller <davem@davemloft.net>
>>
>> show up in the net tree without even having been posted on a
>> block list...
>>
>> At least this kills this ugly thing.
>
> Sorry about that Jens, but at least as you say it's gone now.
Yeah all good now, thanks.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH v2 bpf-next 03/12] selftests/bpf: add BPF_CORE_READ relocatable read macro
From: Song Liu @ 2019-07-30 21:33 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
daniel@iogearbox.net, Yonghong Song, Kernel Team
In-Reply-To: <CAEf4BzakowCqkCkBdGPJCZNU3MpDf1yBhzOXL2pos1tPiUH0mQ@mail.gmail.com>
> On Jul 30, 2019, at 2:26 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> On Tue, Jul 30, 2019 at 2:24 PM Song Liu <songliubraving@fb.com> wrote:
>>
>>
>>
>>> On Jul 30, 2019, at 12:53 PM, Andrii Nakryiko <andriin@fb.com> wrote:
>>>
>>> Add BPF_CORE_READ macro used in tests to do bpf_core_read(), which
>>> automatically captures offset relocation.
>>>
>>> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>>> ---
>>> tools/testing/selftests/bpf/bpf_helpers.h | 19 +++++++++++++++++++
>>> 1 file changed, 19 insertions(+)
>>>
>>> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
>>> index f804f210244e..81bc51293d11 100644
>>> --- a/tools/testing/selftests/bpf/bpf_helpers.h
>>> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
>>> @@ -501,4 +501,23 @@ struct pt_regs;
>>> (void *)(PT_REGS_FP(ctx) + sizeof(ip))); })
>>> #endif
>>>
>>> +/*
>>> + * BPF_CORE_READ abstracts away bpf_probe_read() call and captures offset
>>> + * relocation for source address using __builtin_preserve_access_index()
>>> + * built-in, provided by Clang.
>>> + *
>>> + * __builtin_preserve_access_index() takes as an argument an expression of
>>> + * taking an address of a field within struct/union. It makes compiler emit
>>> + * a relocation, which records BTF type ID describing root struct/union and an
>>> + * accessor string which describes exact embedded field that was used to take
>>> + * an address. See detailed description of this relocation format and
>>> + * semantics in comments to struct bpf_offset_reloc in libbpf_internal.h.
>>> + *
>>> + * This relocation allows libbpf to adjust BPF instruction to use correct
>>> + * actual field offset, based on target kernel BTF type that matches original
>>> + * (local) BTF, used to record relocation.
>>> + */
>>> +#define BPF_CORE_READ(dst, src) \
>>> + bpf_probe_read(dst, sizeof(*src), __builtin_preserve_access_index(src))
>>
>> We should use "sizeof(*(src))"
>>
>
> Good point. Also (dst) instead of just (dst). Will update.
I think dst as-is is fine. "," is the very last in precedence list.
^ permalink raw reply
* Re: [net-next 01/13] net/mlx5e: Print a warning when LRO feature is dropped or not allowed
From: Willem de Bruijn @ 2019-07-30 21:34 UTC (permalink / raw)
To: Saeed Mahameed; +Cc: Huy Nguyen, davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <cb43e9dadb8e48d27df8f08464bf40f7a81eafe9.camel@mellanox.com>
On Tue, Jul 30, 2019 at 4:08 PM Saeed Mahameed <saeedm@mellanox.com> wrote:
>
> On Tue, 2019-07-30 at 11:52 -0400, Willem de Bruijn wrote:
> > On Mon, Jul 29, 2019 at 7:50 PM Saeed Mahameed <saeedm@mellanox.com>
> > wrote:
> > > From: Huy Nguyen <huyn@mellanox.com>
> > >
> > > When user enables LRO via ethtool and if the RQ mode is legacy,
> > > mlx5e_fix_features drops the request without any explanation.
> > > Add netdev_warn to cover this case.
> > >
> > > Fixes: 6c3a823e1e9c ("net/mlx5e: RX, Remove HW LRO support in
> > > legacy RQ")
> > > Signed-off-by: Huy Nguyen <huyn@mellanox.com>
> > > Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> > > ---
> > > drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> > > b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> > > index 47eea6b3a1c3..776eb46d263d 100644
> > > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> > > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> > > @@ -3788,9 +3788,10 @@ static netdev_features_t
> > > mlx5e_fix_features(struct net_device *netdev,
> > > netdev_warn(netdev, "Dropping C-tag vlan
> > > stripping offload due to S-tag vlan\n");
> > > }
> > > if (!MLX5E_GET_PFLAG(params, MLX5E_PFLAG_RX_STRIDING_RQ)) {
> > > - features &= ~NETIF_F_LRO;
> > > - if (params->lro_en)
> > > + if (features & NETIF_F_LRO) {
> > > netdev_warn(netdev, "Disabling LRO, not
> > > supported in legacy RQ\n");
> >
> > This warns about "Disabling LRO" on an enable request?
> >
>
> no, this warning appears only when lro is already enabled and might
> conflict with any other feature requested by user (hence
> mlx5e_fix_features), e.g when moving away from striding rq in this
> example, we will force lro to off.
Ok. The previous commit mentioned "totally remove LRO support in
Legacy RQ". This handles the additional case when moving a queue into
legacy mode that still had LRO enabled. I see.
>
>
> > More fundamentally, it appears that the device does not advertise
> > the feature as configurable in netdev_hw_features as of commit
> > 6c3a823e1e9c ("net/mlx5e: RX, Remove HW LRO support in
> > legacy RQ"), so shouldn't this be caught by the device driver
> > independent ethtool code?
>
> when hw doesn't support MLX5E_PFLAG_RX_STRIDING_RQ then yes, you will
> never hit this code path, but when hw does support
> MLX5E_PFLAG_RX_STRIDING_RQ and you want to turn striding rq off, then
> lro will be forced to off (if it was enabled in first space) and a
> warning msg will be shown.
Got it, thanks.
^ permalink raw reply
* Re: [net 1/1] tipc: fix unitilized skb list crash
From: David Miller @ 2019-07-30 21:40 UTC (permalink / raw)
To: jon.maloy
Cc: netdev, tung.q.nguyen, hoang.h.le, lxin, shuali, ying.xue,
tipc-discussion
In-Reply-To: <1564510750-19531-1-git-send-email-jon.maloy@ericsson.com>
From: Jon Maloy <jon.maloy@ericsson.com>
Date: Tue, 30 Jul 2019 20:19:10 +0200
> Our test suite somtimes provokes the following crash:
>
> Description of problem:
...
> The reason is that the skb list tipc_socket::mc_method.deferredq only
> is initialized for connectionless sockets, while nothing stops arriving
> multicast messages from being filtered by connection oriented sockets,
> with subsequent access to the said list.
>
> We fix this by initializing the list unconditionally at socket creation.
> This eliminates the crash, while the message still is dropped further
> down in tipc_sk_filter_rcv() as it should be.
>
> Reported-by: Li Shuang <shuali@redhat.com>
> Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
Applied and queued up for -stable, thank you.
^ permalink raw reply
* Re: [PATCH v5 09/29] compat_ioctl: pppoe: fix PPPOEIOCSFWD handling
From: David Miller @ 2019-07-30 21:42 UTC (permalink / raw)
To: arnd
Cc: viro, linux-fsdevel, linux-kernel, g.nault, mostrows, xeb,
jchapman, netdev
In-Reply-To: <20190730192552.4014288-10-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 30 Jul 2019 21:25:20 +0200
> Support for handling the PPPOEIOCSFWD ioctl in compat mode was added in
> linux-2.5.69 along with hundreds of other commands, but was always broken
> sincen only the structure is compatible, but the command number is not,
> due to the size being sizeof(size_t), or at first sizeof(sizeof((struct
> sockaddr_pppox)), which is different on 64-bit architectures.
>
> Guillaume Nault adds:
>
> And the implementation was broken until 2016 (see 29e73269aa4d ("pppoe:
> fix reference counting in PPPoE proxy")), and nobody ever noticed. I
> should probably have removed this ioctl entirely instead of fixing it.
> Clearly, it has never been used.
>
> Fix it by adding a compat_ioctl handler for all pppoe variants that
> translates the command number and then calls the regular ioctl function.
>
> All other ioctl commands handled by pppoe are compatible between 32-bit
> and 64-bit, and require compat_ptr() conversion.
>
> This should apply to all stable kernels.
>
> Acked-by: Guillaume Nault <g.nault@alphalink.fr>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied and queued up for -stable, thanks everyone.
^ permalink raw reply
* Re: [PATCH v5 12/29] compat_ioctl: move drivers to compat_ptr_ioctl
From: David Miller @ 2019-07-30 21:43 UTC (permalink / raw)
To: arnd
Cc: viro, linux-fsdevel, linux-kernel, gregkh, mst, jarkko.sakkinen,
jgg, jkosina, stefanha, linux-integrity, linux1394-devel,
linux-usb, linux-input, linux-stm32, linux-arm-kernel, linux-mtd,
netdev, devel, kvm, virtualization, ceph-devel
In-Reply-To: <20190730195227.742215-1-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 30 Jul 2019 21:50:28 +0200
> Each of these drivers has a copy of the same trivial helper function to
> convert the pointer argument and then call the native ioctl handler.
>
> We now have a generic implementation of that, so use it.
>
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Reviewed-by: Jason Gunthorpe <jgg@mellanox.com>
> Reviewed-by: Jiri Kosina <jkosina@suse.cz>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
I assume this has to go via your series, thus:
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH v4 net-next 13/19] ionic: Add initial ethtool support
From: Shannon Nelson @ 2019-07-30 21:51 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, davem
In-Reply-To: <20190725133506.GD21952@lunn.ch>
On 7/25/19 6:35 AM, Andrew Lunn wrote:
>> +static int ionic_get_module_eeprom(struct net_device *netdev,
>> + struct ethtool_eeprom *ee,
>> + u8 *data)
>> +{
>> + struct lif *lif = netdev_priv(netdev);
>> + struct ionic_dev *idev = &lif->ionic->idev;
>> + struct xcvr_status *xcvr;
>> + u32 len;
>> +
>> + /* The NIC keeps the module prom up-to-date in the DMA space
>> + * so we can simply copy the module bytes into the data buffer.
>> + */
>> + xcvr = &idev->port_info->status.xcvr;
>> + len = min_t(u32, sizeof(xcvr->sprom), ee->len);
>> + memcpy(data, xcvr->sprom, len);
>> +
>> + return 0;
>> +}
> Is the firmware doing this DMA update atomically? The diagnostic
> values are u16s. Is there any chance we do this memcpy at the same
> time the DMA is active and we get a mix of old and new data?
>
> Often in cases like this you do the copy twice and ensure you get the
> same values each time. If not, keep repeating the copy until you do
> get the same values twice.
Regardless of how the structs are all aligned and our PCI block does
large writes, I can see how an unoptimized memcpy() that is doing
byte-by-byte copy rather than by words might result in a mangled value.
I think this is the only buffer that may be susceptible to this. Sure,
doing a double copy should work here.
sln
^ permalink raw reply
* Re: [PATCH bpf-next v2] tools: bpftool: add support for reporting the effective cgroup progs
From: Takshak Chahande @ 2019-07-30 22:01 UTC (permalink / raw)
To: Jakub Kicinski
Cc: alexei.starovoitov@gmail.com, daniel@iogearbox.net,
netdev@vger.kernel.org, bpf@vger.kernel.org,
oss-drivers@netronome.com, Kernel Team, Quentin Monnet
In-Reply-To: <20190730210300.13113-1-jakub.kicinski@netronome.com>
Jakub Kicinski <jakub.kicinski@netronome.com> wrote on Tue [2019-Jul-30 14:03:00 -0700]:
> Takshak said in the original submission:
>
> With different bpf attach_flags available to attach bpf programs specially
> with BPF_F_ALLOW_OVERRIDE and BPF_F_ALLOW_MULTI, the list of effective
> bpf-programs available to any sub-cgroups really needs to be available for
> easy debugging.
>
> Using BPF_F_QUERY_EFFECTIVE flag, one can get the list of not only attached
> bpf-programs to a cgroup but also the inherited ones from parent cgroup.
>
> So a new option is introduced to use BPF_F_QUERY_EFFECTIVE query flag here
> to list all the effective bpf-programs available for execution at a specified
> cgroup.
>
> Reused modified test program test_cgroup_attach from tools/testing/selftests/bpf:
> # ./test_cgroup_attach
>
> With old bpftool:
>
> # bpftool cgroup show /sys/fs/cgroup/cgroup-test-work-dir/cg1/
> ID AttachType AttachFlags Name
> 271 egress multi pkt_cntr_1
> 272 egress multi pkt_cntr_2
>
> Attached new program pkt_cntr_4 in cg2 gives following:
>
> # bpftool cgroup show /sys/fs/cgroup/cgroup-test-work-dir/cg1/cg2
> ID AttachType AttachFlags Name
> 273 egress override pkt_cntr_4
>
> And with new "effective" option it shows all effective programs for cg2:
>
> # bpftool cgroup show /sys/fs/cgroup/cgroup-test-work-dir/cg1/cg2 effective
> ID AttachType AttachFlags Name
> 273 egress override pkt_cntr_4
> 271 egress override pkt_cntr_1
> 272 egress override pkt_cntr_2
>
> Compared to original submission use a local flag instead of global
> option.
>
> We need to clear query_flags on every command, in case batch mode
> wants to use varying settings.
>
> v2: (Takshak)
> - forbid duplicated flags;
> - fix cgroup path freeing.
>
> Signed-off-by: Takshak Chahande <ctakshak@fb.com>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
> ---
> .../bpftool/Documentation/bpftool-cgroup.rst | 16 +++-
> tools/bpf/bpftool/bash-completion/bpftool | 15 ++--
> tools/bpf/bpftool/cgroup.c | 83 ++++++++++++-------
> 3 files changed, 76 insertions(+), 38 deletions(-)
>
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
> index 585f270c2d25..06a28b07787d 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
> @@ -20,8 +20,8 @@ SYNOPSIS
> CGROUP COMMANDS
> ===============
>
> -| **bpftool** **cgroup { show | list }** *CGROUP*
> -| **bpftool** **cgroup tree** [*CGROUP_ROOT*]
> +| **bpftool** **cgroup { show | list }** *CGROUP* [**effective**]
> +| **bpftool** **cgroup tree** [*CGROUP_ROOT*] [**effective**]
> | **bpftool** **cgroup attach** *CGROUP* *ATTACH_TYPE* *PROG* [*ATTACH_FLAGS*]
> | **bpftool** **cgroup detach** *CGROUP* *ATTACH_TYPE* *PROG*
> | **bpftool** **cgroup help**
> @@ -35,13 +35,17 @@ CGROUP COMMANDS
>
> DESCRIPTION
> ===========
> - **bpftool cgroup { show | list }** *CGROUP*
> + **bpftool cgroup { show | list }** *CGROUP* [**effective**]
> List all programs attached to the cgroup *CGROUP*.
>
> Output will start with program ID followed by attach type,
> attach flags and program name.
>
> - **bpftool cgroup tree** [*CGROUP_ROOT*]
> + If **effective** is specified retrieve effective programs that
> + will execute for events within a cgroup. This includes
> + inherited along with attached ones.
> +
> + **bpftool cgroup tree** [*CGROUP_ROOT*] [**effective**]
> Iterate over all cgroups in *CGROUP_ROOT* and list all
> attached programs. If *CGROUP_ROOT* is not specified,
> bpftool uses cgroup v2 mountpoint.
> @@ -50,6 +54,10 @@ DESCRIPTION
> commands: it starts with absolute cgroup path, followed by
> program ID, attach type, attach flags and program name.
>
> + If **effective** is specified retrieve effective programs that
> + will execute for events within a cgroup. This includes
> + inherited along with attached ones.
> +
> **bpftool cgroup attach** *CGROUP* *ATTACH_TYPE* *PROG* [*ATTACH_FLAGS*]
> Attach program *PROG* to the cgroup *CGROUP* with attach type
> *ATTACH_TYPE* and optional *ATTACH_FLAGS*.
> diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
> index 6b961a5ed100..df16c5415444 100644
> --- a/tools/bpf/bpftool/bash-completion/bpftool
> +++ b/tools/bpf/bpftool/bash-completion/bpftool
> @@ -710,12 +710,15 @@ _bpftool()
> ;;
> cgroup)
> case $command in
> - show|list)
> - _filedir
> - return 0
> - ;;
> - tree)
> - _filedir
> + show|list|tree)
> + case $cword in
> + 3)
> + _filedir
> + ;;
> + 4)
> + COMPREPLY=( $( compgen -W 'effective' -- "$cur" ) )
> + ;;
> + esac
> return 0
> ;;
> attach|detach)
> diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
> index f3c05b08c68c..44352b5aca85 100644
> --- a/tools/bpf/bpftool/cgroup.c
> +++ b/tools/bpf/bpftool/cgroup.c
> @@ -29,6 +29,8 @@
> " recvmsg4 | recvmsg6 | sysctl |\n" \
> " getsockopt | setsockopt }"
>
> +static unsigned int query_flags;
> +
> static const char * const attach_type_strings[] = {
> [BPF_CGROUP_INET_INGRESS] = "ingress",
> [BPF_CGROUP_INET_EGRESS] = "egress",
> @@ -107,7 +109,8 @@ static int count_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type)
> __u32 prog_cnt = 0;
> int ret;
>
> - ret = bpf_prog_query(cgroup_fd, type, 0, NULL, NULL, &prog_cnt);
> + ret = bpf_prog_query(cgroup_fd, type, query_flags, NULL,
> + NULL, &prog_cnt);
> if (ret)
> return -1;
>
> @@ -125,8 +128,8 @@ static int show_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type,
> int ret;
>
> prog_cnt = ARRAY_SIZE(prog_ids);
> - ret = bpf_prog_query(cgroup_fd, type, 0, &attach_flags, prog_ids,
> - &prog_cnt);
> + ret = bpf_prog_query(cgroup_fd, type, query_flags, &attach_flags,
> + prog_ids, &prog_cnt);
> if (ret)
> return ret;
>
> @@ -158,20 +161,34 @@ static int show_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type,
> static int do_show(int argc, char **argv)
> {
> enum bpf_attach_type type;
> + const char *path;
> int cgroup_fd;
> int ret = -1;
>
> - if (argc < 1) {
> - p_err("too few parameters for cgroup show");
> - goto exit;
> - } else if (argc > 1) {
> - p_err("too many parameters for cgroup show");
> - goto exit;
> + query_flags = 0;
> +
> + if (!REQ_ARGS(1))
> + return -1;
> + path = GET_ARG();
> +
> + while (argc) {
> + if (is_prefix(*argv, "effective")) {
> + if (query_flags & BPF_F_QUERY_EFFECTIVE) {
> + p_err("duplicated argument: %s", *argv);
> + return -1;
> + }
> + query_flags |= BPF_F_QUERY_EFFECTIVE;
> + NEXT_ARG();
> + } else {
> + p_err("expected no more arguments, 'effective', got: '%s'?",
> + *argv);
> + return -1;
> + }
> }
>
> - cgroup_fd = open(argv[0], O_RDONLY);
> + cgroup_fd = open(path, O_RDONLY);
> if (cgroup_fd < 0) {
> - p_err("can't open cgroup %s", argv[0]);
> + p_err("can't open cgroup %s", path);
> goto exit;
> }
>
> @@ -294,26 +311,37 @@ static char *find_cgroup_root(void)
>
> static int do_show_tree(int argc, char **argv)
> {
> - char *cgroup_root;
> + char *cgroup_root, *cgroup_alloced = NULL;
> int ret;
>
> - switch (argc) {
> - case 0:
> - cgroup_root = find_cgroup_root();
> - if (!cgroup_root) {
> + query_flags = 0;
> +
> + if (!argc) {
> + cgroup_alloced = find_cgroup_root();
> + if (!cgroup_alloced) {
> p_err("cgroup v2 isn't mounted");
> return -1;
> }
> - break;
> - case 1:
> - cgroup_root = argv[0];
> - break;
> - default:
> - p_err("too many parameters for cgroup tree");
> - return -1;
> + cgroup_root = cgroup_alloced;
> + } else {
> + cgroup_root = GET_ARG();
> +
> + while (argc) {
> + if (is_prefix(*argv, "effective")) {
> + if (query_flags & BPF_F_QUERY_EFFECTIVE) {
> + p_err("duplicated argument: %s", *argv);
> + return -1;
> + }
> + query_flags |= BPF_F_QUERY_EFFECTIVE;
> + NEXT_ARG();
> + } else {
> + p_err("expected no more arguments, 'effective', got: '%s'?",
> + *argv);
> + return -1;
> + }
> + }
> }
>
> -
> if (json_output)
> jsonw_start_array(json_wtr);
> else
> @@ -338,8 +366,7 @@ static int do_show_tree(int argc, char **argv)
> if (json_output)
> jsonw_end_array(json_wtr);
>
> - if (argc == 0)
> - free(cgroup_root);
> + free(cgroup_alloced);
>
> return ret;
> }
> @@ -459,8 +486,8 @@ static int do_help(int argc, char **argv)
> }
>
> fprintf(stderr,
> - "Usage: %s %s { show | list } CGROUP\n"
> - " %s %s tree [CGROUP_ROOT]\n"
> + "Usage: %s %s { show | list } CGROUP [**effective**]\n"
> + " %s %s tree [CGROUP_ROOT] [**effective**]\n"
> " %s %s attach CGROUP ATTACH_TYPE PROG [ATTACH_FLAGS]\n"
> " %s %s detach CGROUP ATTACH_TYPE PROG\n"
> " %s %s help\n"
> --
> 2.21.0
>
Thanks for v2; looks good.
Reviewed-by: Takshak Chahande <ctakshak@fb.com>
^ permalink raw reply
* Re: [PATCH v3] net: dsa: qca8k: enable port flow control
From: David Miller @ 2019-07-30 22:08 UTC (permalink / raw)
To: xiaofeis
Cc: vkoul, netdev, andrew, linux-arm-msm, bjorn.andersson,
vivien.didelot, f.fainelli, niklas.cassel, xiazha
In-Reply-To: <1564275470-52666-1-git-send-email-xiaofeis@codeaurora.org>
From: xiaofeis <xiaofeis@codeaurora.org>
Date: Sun, 28 Jul 2019 08:57:50 +0800
> Set phy device advertising to enable MAC flow control.
>
> Signed-off-by: Xiaofei Shen <xiaofeis@codeaurora.org>
I've read the discussion over a few times and if internal PHY is the
only thing supported, and the specific setup this driver supports uses
internal signalling to sync the PHY and MAC settings I guess this is
OK although suboptimal.
So applied, thanks.
Thanks.
^ permalink raw reply
* Re: [PATCH V4 0/3] net: dsa: ksz: Add Microchip KSZ87xx support
From: David Miller @ 2019-07-30 22:14 UTC (permalink / raw)
To: marex; +Cc: netdev, andrew, f.fainelli, Tristram.Ha, vivien.didelot,
woojung.huh
In-Reply-To: <20190729174947.10103-1-marex@denx.de>
From: Marek Vasut <marex@denx.de>
Date: Mon, 29 Jul 2019 19:49:44 +0200
> This series adds support for Microchip KSZ87xx switches, which are
> slightly simpler compared to KSZ9xxx .
>
> Signed-off-by: Marek Vasut <marex@denx.de>
Series applied to net-next, thank you.
^ 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