* [PATCH bpf-next] selftests/bpf: enable (uncomment) all tests in test_libbpf.sh
From: Quentin Monnet @ 2018-10-20 22:00 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: netdev, oss-drivers, Quentin Monnet, Jesper Dangaard Brouer
libbpf is now able to load successfully test_l4lb_noinline.o and
samples/bpf/tracex3_kern.o, so we can uncomment related tests from
test_libbpf.c and remove the associated "TODO"s.
It is also trivial to fix test_xdp_noinline.o so that it provides a
version and can be loaded. Fix it and uncomment this test as well.
For the record, the error message obtainted with tracex3_kern.o was
fixed by commit e3d91b0ca523 ("tools/libbpf: handle issues with bpf ELF
objects containing .eh_frames")
I have not been abled to reproduce the "libbpf: incorrect bpf_call
opcode" error for test_l4lb_noinline.o, even with the version of libbpf
present at the time when test_libbpf.sh and test_libbpf_open.c were
created.
Cc: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
---
tools/testing/selftests/bpf/test_libbpf.sh | 12 +++---------
tools/testing/selftests/bpf/test_xdp_meta.c | 2 ++
2 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_libbpf.sh b/tools/testing/selftests/bpf/test_libbpf.sh
index 156d89f1edcc..a426f28163a5 100755
--- a/tools/testing/selftests/bpf/test_libbpf.sh
+++ b/tools/testing/selftests/bpf/test_libbpf.sh
@@ -33,17 +33,11 @@ trap exit_handler 0 2 3 6 9
libbpf_open_file test_l4lb.o
-# TODO: fix libbpf to load noinline functions
-# [warning] libbpf: incorrect bpf_call opcode
-#libbpf_open_file test_l4lb_noinline.o
+libbpf_open_file test_l4lb_noinline.o
-# TODO: fix test_xdp_meta.c to load with libbpf
-# [warning] libbpf: test_xdp_meta.o doesn't provide kernel version
-#libbpf_open_file test_xdp_meta.o
+libbpf_open_file test_xdp_meta.o
-# TODO: fix libbpf to handle .eh_frame
-# [warning] libbpf: relocation failed: no section(10)
-#libbpf_open_file ../../../../samples/bpf/tracex3_kern.o
+libbpf_open_file ../../../../samples/bpf/tracex3_kern.o
# Success
exit 0
diff --git a/tools/testing/selftests/bpf/test_xdp_meta.c b/tools/testing/selftests/bpf/test_xdp_meta.c
index 8d0182650653..2f42de66e2bb 100644
--- a/tools/testing/selftests/bpf/test_xdp_meta.c
+++ b/tools/testing/selftests/bpf/test_xdp_meta.c
@@ -8,6 +8,8 @@
#define round_up(x, y) ((((x) - 1) | __round_mask(x, y)) + 1)
#define ctx_ptr(ctx, mem) (void *)(unsigned long)ctx->mem
+int _version SEC("version") = 1;
+
SEC("t")
int ing_cls(struct __sk_buff *ctx)
{
--
2.7.4
^ permalink raw reply related
* [PATCH bpf-next] selftests/bpf: fix return value comparison for tests in test_libbpf.sh
From: Quentin Monnet @ 2018-10-20 21:58 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, oss-drivers, Quentin Monnet
The return value for each test in test_libbpf.sh is compared with
if (( $? == 0 )) ; then ...
This works well with bash, but not with dash, that /bin/sh is aliased to
on some systems (such as Ubuntu).
Let's replace this comparison by something that works on both shells.
Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
tools/testing/selftests/bpf/test_libbpf.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/test_libbpf.sh b/tools/testing/selftests/bpf/test_libbpf.sh
index d97dc914cd49..156d89f1edcc 100755
--- a/tools/testing/selftests/bpf/test_libbpf.sh
+++ b/tools/testing/selftests/bpf/test_libbpf.sh
@@ -6,7 +6,7 @@ export TESTNAME=test_libbpf
# Determine selftest success via shell exit code
exit_handler()
{
- if (( $? == 0 )); then
+ if [ $? -eq 0 ]; then
echo "selftests: $TESTNAME [PASS]";
else
echo "$TESTNAME: failed at file $LAST_LOADED" 1>&2
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net v2] net/sched: act_gact: properly init 'goto chain'
From: Davide Caratti @ 2018-10-20 21:40 UTC (permalink / raw)
To: Cong Wang
Cc: Jamal Hadi Salim, Jiri Pirko, David Miller,
Linux Kernel Network Developers
In-Reply-To: <CAM_iQpXtNBR9ELyexTODSU9v8+FPD_Q1oORqonQ69hK=0dBhWg@mail.gmail.com>
hello Cong and Jamal,
On Fri, 2018-10-19 at 13:40 -0700, Cong Wang wrote:
> On Thu, Oct 18, 2018 at 8:30 AM Davide Caratti <dcaratti@redhat.com> wrote:
> > The alternative is, we systematically forbid usage of 'goto chain' in
> > tcfg_paction, so that:
> >
> > # tc f a dev v0 egress matchall action <whatever> random determ goto chain 4 5
> >
> > is systematically rejected with -EINVAL. This comand never worked, so we
> > are not breaking anything in userspace.
> This is exactly why I asked you if we really need to support it. :)
>
> If no one finds it useful, disallowing it is a good solution here, as
> we don't need
> to introduce any additional code to handle filter chains.
On Thu, 2018-10-18 at 08:52 -0400, Jamal Hadi Salim wrote:
> Rejection is a good solution[1].
> Would be helpful to set an ext_ack to something like
> "only one goto chain is supported currently"
OK to forbid 'goto chain' on fallback actions for gact and police: I just
sent out a small series for that, feedbacks are welcome.
@David: this patch is no more needed, it can be dropped from patchwork.
thanks!
regards,
--
davide
^ permalink raw reply
* [PATCH net 4/4] tc-tests: test denial of 'goto chain' for exceed traffic in police.json
From: Davide Caratti @ 2018-10-20 21:33 UTC (permalink / raw)
To: Jiri Pirko, Cong Wang, Jamal Hadi Salim, David S. Miller, netdev
In-Reply-To: <cover.1540070509.git.dcaratti@redhat.com>
add test to verify if act_police forbids 'goto chain' control actions for
'exceed' traffic.
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
.../tc-testing/tc-tests/actions/police.json | 24 +++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/police.json b/tools/testing/selftests/tc-testing/tc-tests/actions/police.json
index 30f9b54bd666..4086a50a670e 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/police.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/police.json
@@ -715,5 +715,29 @@
"teardown": [
"$TC actions flush action police"
]
+ },
+ {
+ "id": "b48b",
+ "name": "Add police action with exceed goto chain control action",
+ "category": [
+ "actions",
+ "police"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action police",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action police rate 1mbit burst 1k conform-exceed pass / goto chain 42",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions ls action police",
+ "matchPattern": "action order [0-9]*: police 0x1 rate 1Mbit burst 1Kb mtu 2Kb action pass/goto chain 42",
+ "matchCount": "0",
+ "teardown": [
+ "$TC actions flush action police"
+ ]
}
]
--
2.17.1
^ permalink raw reply related
* [PATCH net 3/4] tc-tests: test denial of 'goto chain' on 'random' traffic in gact.json
From: Davide Caratti @ 2018-10-20 21:33 UTC (permalink / raw)
To: Jiri Pirko, Cong Wang, Jamal Hadi Salim, David S. Miller, netdev
In-Reply-To: <cover.1540070509.git.dcaratti@redhat.com>
add test to verify if act_gact forbids 'goto chain' control actions on
'random' traffic in gact.json.
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
.../tc-testing/tc-tests/actions/gact.json | 24 +++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json b/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json
index 68c91023cdb9..89189a03ce3d 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json
@@ -536,5 +536,29 @@
"matchPattern": "^[ \t]+index [0-9]+ ref",
"matchCount": "0",
"teardown": []
+ },
+ {
+ "id": "8e47",
+ "name": "Add gact action with random determ goto chain control action",
+ "category": [
+ "actions",
+ "gact"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action gact",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action pass random determ goto chain 1 2 index 90",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions list action gact",
+ "matchPattern": "action order [0-9]*: gact action pass random type determ goto chain 1 val 2.*index 90 ref",
+ "matchCount": "0",
+ "teardown": [
+ "$TC actions flush action gact"
+ ]
}
]
--
2.17.1
^ permalink raw reply related
* [PATCH net 2/4] net/sched: act_police: disallow 'goto chain' on fallback control action
From: Davide Caratti @ 2018-10-20 21:33 UTC (permalink / raw)
To: Jiri Pirko, Cong Wang, Jamal Hadi Salim, David S. Miller, netdev
In-Reply-To: <cover.1540070509.git.dcaratti@redhat.com>
in the following command:
# tc action add action police rate <r> burst <b> conform-exceed <c1>/<c2>
'goto chain x' is allowed only for c1: setting it for c2 makes the kernel
crash with NULL pointer dereference, since TC core doesn't initialize the
chain handle.
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/sched/act_police.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 5d8bfa878477..3b793393efd1 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -150,6 +150,16 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
goto failure;
}
+ if (tb[TCA_POLICE_RESULT]) {
+ police->tcfp_result = nla_get_u32(tb[TCA_POLICE_RESULT]);
+ if (TC_ACT_EXT_CMP(police->tcfp_result, TC_ACT_GOTO_CHAIN)) {
+ NL_SET_ERR_MSG(extack,
+ "goto chain not allowed on fallback");
+ err = -EINVAL;
+ goto failure;
+ }
+ }
+
spin_lock_bh(&police->tcf_lock);
/* No failure allowed after this point */
police->tcfp_mtu = parm->mtu;
@@ -173,8 +183,6 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
police->peak_present = false;
}
- if (tb[TCA_POLICE_RESULT])
- police->tcfp_result = nla_get_u32(tb[TCA_POLICE_RESULT]);
police->tcfp_burst = PSCHED_TICKS2NS(parm->burst);
police->tcfp_toks = police->tcfp_burst;
if (police->peak_present) {
--
2.17.1
^ permalink raw reply related
* [PATCH net 1/4] net/sched: act_gact: disallow 'goto chain' on fallback control action
From: Davide Caratti @ 2018-10-20 21:33 UTC (permalink / raw)
To: Jiri Pirko, Cong Wang, Jamal Hadi Salim, David S. Miller, netdev
In-Reply-To: <cover.1540070509.git.dcaratti@redhat.com>
in the following command:
# tc action add action <c1> random <rand_type> <c2> <rand_param>
'goto chain x' is allowed only for c1: setting it for c2 makes the kernel
crash with NULL pointer dereference, since TC core doesn't initialize the
chain handle.
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/sched/act_gact.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index cd1d9bd32ef9..505138047e5c 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -88,6 +88,11 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla,
p_parm = nla_data(tb[TCA_GACT_PROB]);
if (p_parm->ptype >= MAX_RAND)
return -EINVAL;
+ if (TC_ACT_EXT_CMP(p_parm->paction, TC_ACT_GOTO_CHAIN)) {
+ NL_SET_ERR_MSG(extack,
+ "goto chain not allowed on fallback");
+ return -EINVAL;
+ }
}
#endif
--
2.17.1
^ permalink raw reply related
* [PATCH net 0/4] net/sched: forbid 'goto_chain' on fallback actions
From: Davide Caratti @ 2018-10-20 21:33 UTC (permalink / raw)
To: Jiri Pirko, Cong Wang, Jamal Hadi Salim, David S. Miller, netdev
the following command:
# tc actions add action police rate 1mbit burst 1k conform-exceed \
> pass / goto chain 42
generates a NULL pointer dereference when packets exceed the configured
rate. Similarly, the following command:
# tc actions add action pass random determ goto chain 42 2
makes the kernel crash with NULL dereference when the first packet does
not match the 'pass' action.
gact and police allow users to specify a fallback control action, that is
stored in the action private data. 'goto chain x' never worked for these
cases, since a->goto_chain handle was never initialized. There is only one
goto_chain handle per TC action, and it is designed to be non-NULL only if
tcf_action contains a 'goto chain' command. So, let's forbid 'goto chain'
on fallback actions.
Patch 1/4 and 2/4 change the .init() functions of police and gact, to let
them return an error when users try to set 'goto chain x' in the fallback
action. Patch 3/4 and 4/4 add TDC selftest coverage to this new behavior.
Davide Caratti (4):
net/sched: act_gact: disallow 'goto chain' on fallback control action
net/sched: act_police: disallow 'goto chain' on fallback control
action
tc-tests: test denial of 'goto chain' on 'random' traffic in gact.json
tc-tests: test denial of 'goto chain' for exceed traffic in
police.json
net/sched/act_gact.c | 5 ++++
net/sched/act_police.c | 12 ++++++++--
.../tc-testing/tc-tests/actions/gact.json | 24 +++++++++++++++++++
.../tc-testing/tc-tests/actions/police.json | 24 +++++++++++++++++++
4 files changed, 63 insertions(+), 2 deletions(-)
--
2.17.1
^ permalink raw reply
* Re: [GIT] Networking
From: David Miller @ 2018-10-21 5:26 UTC (permalink / raw)
To: gregkh; +Cc: akpm, netdev, linux-kernel
In-Reply-To: <20181020.154737.106692150032143674.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Sat, 20 Oct 2018 15:47:37 -0700 (PDT)
>
> A few straggler bug fixes:
>
> 1) Fix indexing of multi-pass dumps of ipv6 addresses, from David
> Ahern.
>
> 2) Revert RCU locking change for bonding netpoll, causes worse problems
> than it solves.
>
> 3) pskb_trim_rcsum_slow() doesn't handle odd trim offsets, resulting in
> erroneous bad hw checksum triggers with CHECKSUM_COMPLETE devices.
> From Dimitris Michailidis.
>
> Please pull, thanks a lot!
Great, I appended one more bug fix to this pull request, a revert to
some neighbour code changes that adjust notifications in a way that
confuses some apps.
====================
>From d2fb4fb8ee91c1b8a1dbba6afda6f5ed2eb28bdc Mon Sep 17 00:00:00 2001
From: Roopa Prabhu <roopa@cumulusnetworks.com>
Date: Sat, 20 Oct 2018 18:09:31 -0700
Subject: [PATCH] Revert "neighbour: force neigh_invalidate when NUD_FAILED
update is from admin"
This reverts commit 8e326289e3069dfc9fa9c209924668dd031ab8ef.
This patch results in unnecessary netlink notification when one
tries to delete a neigh entry already in NUD_FAILED state. Found
this with a buggy app that tries to delete a NUD_FAILED entry
repeatedly. While the notification issue can be fixed with more
checks, adding more complexity here seems unnecessary. Also,
recent tests with other changes in the neighbour code have
shown that the INCOMPLETE and PROBE checks are good enough for
the original issue.
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/core/neighbour.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 91592fceeaad..4e07824eec5e 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -1148,8 +1148,7 @@ int neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new,
neigh->nud_state = new;
err = 0;
notify = old & NUD_VALID;
- if (((old & (NUD_INCOMPLETE | NUD_PROBE)) ||
- (flags & NEIGH_UPDATE_F_ADMIN)) &&
+ if ((old & (NUD_INCOMPLETE | NUD_PROBE)) &&
(new & NUD_FAILED)) {
neigh_invalidate(neigh);
notify = 1;
--
2.17.2
^ permalink raw reply related
* [PATCH] net: ethernet:fec: Consistently use SPEED_ prefix
From: Andrew Lunn @ 2018-10-20 20:48 UTC (permalink / raw)
To: David Miller; +Cc: clabbe, netdev, Florian Fainelli, hkallweit1, Andrew Lunn
All other calls to phy_set_max_speed() use the SPEED_ prefix. Make the
FEC driver follow this common pattern. This makes no different to
generated code since SPEED_1000 is 1000, and SPEED_100 is 100.
Reported-by: Corentin Labbe <clabbe@baylibre.com>
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/ethernet/freescale/fec_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 6db69ba30dcd..b067eaf8b792 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1948,7 +1948,7 @@ static int fec_enet_mii_probe(struct net_device *ndev)
/* mask with MAC supported features */
if (fep->quirks & FEC_QUIRK_HAS_GBIT) {
- phy_set_max_speed(phy_dev, 1000);
+ phy_set_max_speed(phy_dev, SPEED_1000);
phy_remove_link_mode(phy_dev,
ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
#if !defined(CONFIG_M5272)
@@ -1956,7 +1956,7 @@ static int fec_enet_mii_probe(struct net_device *ndev)
#endif
}
else
- phy_set_max_speed(phy_dev, 100);
+ phy_set_max_speed(phy_dev, SPEED_100);
fep->link = 0;
fep->full_duplex = 0;
--
2.19.0
^ permalink raw reply related
* [PATCH net-next] net: phy: phy_support_sym_pause: Clear Asym Pause
From: Andrew Lunn @ 2018-10-20 20:41 UTC (permalink / raw)
To: clabbe, David Miller; +Cc: netdev, Florian Fainelli, hkallweit1, Andrew Lunn
When indicating the MAC supports Symmetric Pause, clear the Asymmetric
Pause bit, which could of been already set is the PHY supports it.
Reported-by: Labbe Corentin <clabbe@baylibre.com>
Fixes: c306ad36184f ("net: ethernet: Add helper for MACs which support pause")
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/phy/phy_device.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 43cb08dcce81..ab33d1777132 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1940,6 +1940,7 @@ EXPORT_SYMBOL(phy_remove_link_mode);
*/
void phy_support_sym_pause(struct phy_device *phydev)
{
+ phydev->supported &= ~SUPPORTED_Asym_Pause;
phydev->supported |= SUPPORTED_Pause;
phydev->advertising = phydev->supported;
}
--
2.19.0
^ permalink raw reply related
* Re: [PATCH] net/mlx5: allocate enough space in
From: Or Gerlitz @ 2018-10-20 20:37 UTC (permalink / raw)
To: Dan Carpenter
Cc: Saeed Mahameed, Paul Blakey, kernel-janitors, Linux Netdev List,
Leon Romanovsky
In-Reply-To: <20181019200638.dea3r4uocbij6d6j@kili.mountain>
On Fri, Oct 19, 2018 at 11:08 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> FDB_MAX_CHAIN is 3. We wanted to allocate enough memory to hold four
> structs but there are missing parentheses so we only allocate enough
> memory for three structs and the first byte of the fourth one.
yeah, seems that we were wrong here and the fix is correct, at some
point I saw Kasan
screams but it was gone later, let me look, thanks for pointing it out.
> Fixes: 328edb499f99 ("net/mlx5: Split FDB fast path prio to multiple namespaces")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/fs_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
> index 67ba4c975d81..9d73eb955f75 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
> @@ -2470,7 +2470,7 @@ static int init_fdb_root_ns(struct mlx5_flow_steering *steering)
> return -ENOMEM;
>
> steering->fdb_sub_ns = kzalloc(sizeof(steering->fdb_sub_ns) *
> - FDB_MAX_CHAIN + 1, GFP_KERNEL);
> + (FDB_MAX_CHAIN + 1), GFP_KERNEL);
> if (!steering->fdb_sub_ns)
> return -ENOMEM;
>
> --
> 2.11.0
>
^ permalink raw reply
* Re: [bpf-next v2 0/3] sockmap, bpf_msg_push_data helper
From: Daniel Borkmann @ 2018-10-20 19:40 UTC (permalink / raw)
To: John Fastabend, ast; +Cc: netdev
In-Reply-To: <1540004211-7647-1-git-send-email-john.fastabend@gmail.com>
On 10/20/2018 04:56 AM, John Fastabend wrote:
> This series adds a new helper bpf_msg_push_data to be used by
> sk_msg programs. The helper can be used to insert extra bytes into
> the message that can then be used by the program as metadata tags
> among other things.
>
> The first patch adds the helper, second patch the libbpf support,
> and last patch updates test_sockmap to run msg_push_data tests.
>
> v2: rebase after queue map and in filter.c convert int -> u32
>
> John Fastabend (3):
> bpf: sk_msg program helper bpf_msg_push_data
> bpf: libbpf support for msg_push_data
> bpf: test_sockmap add options to use msg_push_data
>
> include/linux/skmsg.h | 5 +
> include/uapi/linux/bpf.h | 20 +++-
> net/core/filter.c | 134 ++++++++++++++++++++++++
> tools/include/uapi/linux/bpf.h | 20 +++-
> tools/testing/selftests/bpf/bpf_helpers.h | 2 +
> tools/testing/selftests/bpf/test_sockmap.c | 58 +++++++++-
> tools/testing/selftests/bpf/test_sockmap_kern.h | 97 +++++++++++++----
> 7 files changed, 308 insertions(+), 28 deletions(-)
>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH net-next] r8169: add support for Byte Queue Limits
From: David Miller @ 2018-10-20 19:36 UTC (permalink / raw)
To: hkallweit1; +Cc: nic_swsd, netdev, fw, holger
In-Reply-To: <778bd978-e851-6604-9822-34bde3eb07b9@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Sat, 20 Oct 2018 12:25:27 +0200
> From: Florian Westphal <fw@strlen.de>
> This patch is basically a resubmit of 1e918876853a ("r8169: add support
> for Byte Queue Limits") which was reverted later. The problems causing
> the revert seem to have been fixed in the meantime.
> Only change to the original patch is that the call to
> netdev_reset_queue was moved to rtl8169_tx_clear.
>
> The Tested-by refers to a system using the RTL8168evl chip version.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> Tested-by: Holger Hoffstätte <holger@applied-asynchrony.com>
Applied.
Heiner, I just want to say how happy I am with the work you've done on
the r8169 driver. So many long term issues have been resolved, the
driver uses more and more generic facilities instead of reimplementing
things, and it's much cleaner and easier to maintain now.
Thank you!
^ permalink raw reply
* Re: [PATCH net-next] r8169: handle all interrupt events in the hard irq handler
From: David Miller @ 2018-10-20 19:35 UTC (permalink / raw)
To: hkallweit1; +Cc: nic_swsd, netdev
In-Reply-To: <b37a0812-1c23-3743-eaf6-ca8bdcb1b1ab@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Thu, 18 Oct 2018 22:19:28 +0200
> Having a separate "slow event" handler isn't needed because all
> interrupt events trigger asynchronous activity. And in case of SYSErr
> we have bigger problems than performance anyway.
> This patch also allows to get rid of acking interrupt events in the
> NAPI poll callback.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> This patch will apply to net-next only after 6b839b6cf9ea ("r8169: fix
> NAPI handling under high load") was merged from net to net-next.
Applied.
^ permalink raw reply
* Re: pull request: bluetooth-next 2018-10-20
From: David Miller @ 2018-10-20 19:34 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth, netdev
In-Reply-To: <20181020090342.GA14926@x1c.home>
From: Johan Hedberg <johan.hedberg@gmail.com>
Date: Sat, 20 Oct 2018 12:03:42 +0300
> Here's one more bluetooth-next pull request for the 4.20 kernel.
>
> - Added new USB ID for QCA_ROME controller
> - Added debug trace support from QCA wcn3990 controllers
> - Updated L2CAP to conform to latest Errata Service Release
> - Fix binding to non-removable BCM43430 devices
>
> Please let me know if there are any issues pulling. Thanks.
Pulled, thanks Johan.
^ permalink raw reply
* Re: [PATCH 00/10] Netfilter updates for net-next
From: David Miller @ 2018-10-20 19:33 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <20181020094317.16011-1-pablo@netfilter.org>
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Sat, 20 Oct 2018 11:43:07 +0200
> The following patchset contains Netfilter updates for your net-next tree:
>
> 1) Use lockdep_is_held() in ipset_dereference_protected(), from Lance Roy.
>
> 2) Remove unused variable in cttimeout, from YueHaibing.
>
> 3) Add ttl option for nft_osf, from Fernando Fernandez Mancera.
>
> 4) Use xfrm family to deal with IPv6-in-IPv4 packets from nft_xfrm,
> from Florian Westphal.
>
> 5) Simplify xt_osf_match_packet().
>
> 6) Missing ct helper alias definition in snmp_trap helper, from Taehee Yoo.
>
> 7) Remove unnecessary parameter in nf_flow_table_cleanup(), from Taehee Yoo.
>
> 8) Remove unused variable definitions in nft_{dup,fwd}, from Weongyo Jeong.
>
> 9) Remove empty net/netfilter/nfnetlink_log.h file, from Taehee Yoo.
>
> 10) Revert xt_quota updates remain option due to problems in the listing
> path for 32-bit arches, from Maze.
>
> You can pull these changes from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git
Pulled, thanks.
^ permalink raw reply
* Re: [PATCH iproute2 1/1] DEBUG: Fix make check when need build generate_nlmsg
From: Petr Vorel @ 2018-10-20 19:15 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20180925124956.10565-1-pvorel@suse.cz>
Hi Stephen,
> make check from top level Makefile defines several flags which break
> building generate_nlmsg:
> $ make check
> make -C tools
> gcc -Wall -Wstrict-prototypes -Wmissing-prototypes -Wmissing-declarations -Wold-style-definition -Wformat=2 -O2 -I../include -I../include/uapi -DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\" -DNETNS_RUN_DIR=\"/var/run/netns\" -DNETNS_ETC_DIR=\"/etc/netns\" -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -DHAVE_SETNS -DHAVE_SELINUX -DHAVE_ELF -DHAVE_LIBMNL -I/usr/include/libmnl -DNEED_STRLCPY -DHAVE_LIBCAP ../lib/libutil.a ../lib/libnetlink.a -lselinux -lelf -lmnl -lcap -I../../include -include../../include/uapi/linux/netlink.h -o generate_nlmsg generate_nlmsg.c ../../lib/libnetlink.c -lmnl
> gcc: error: ../lib/libutil.a: No such file or directory
> gcc: error: ../lib/libnetlink.a: No such file or directory
> make[2]: *** [Makefile:5: generate_nlmsg] Error 1
> make[1]: *** [Makefile:40: generate_nlmsg] Error 2
> To fix it reset CFLAGS in sub Makefile and remove LDLIBS entirely (as
> required -lmnl flag was specified in 5dc2204c ("testsuite: add libmnl").
> Fixes: 8804a8c0 ("Makefile: Add check target")
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Hi Stephen,
> I'm sorry for this regression.
> Kind regards,
> Petr
> ---
> testsuite/tools/Makefile | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> diff --git a/testsuite/tools/Makefile b/testsuite/tools/Makefile
> index 7d53d226..e1d9bfef 100644
> --- a/testsuite/tools/Makefile
> +++ b/testsuite/tools/Makefile
> @@ -1,8 +1,9 @@
> # SPDX-License-Identifier: GPL-2.0
> +CFLAGS=
> include ../../config.mk
> generate_nlmsg: generate_nlmsg.c ../../lib/libnetlink.c
> - $(CC) $(CPPFLAGS) $(CFLAGS) $(LDLIBS) $(EXTRA_CFLAGS) -I../../include -include../../include/uapi/linux/netlink.h -o $@ $^ -lmnl
> + $(CC) $(CPPFLAGS) $(CFLAGS) $(EXTRA_CFLAGS) -I../../include -include../../include/uapi/linux/netlink.h -o $@ $^ -lmnl
> clean:
> rm -f generate_nlmsg
ping, please. Patch is in state "accepted in patchwork [1], but not merged.
Subject should be "testsuite: Fix make check when need build generate_nlmsg".
Kind regards,
Petr
[1] https://patchwork.ozlabs.org/patch/974391/
^ permalink raw reply
* [PATCH 2/2] net: bpfilter: Set user mode helper's command line
From: Olivier Brunel @ 2018-10-20 17:39 UTC (permalink / raw)
To: Network Development
Cc: Alexei Starovoitov, Daniel Borkmann, Luis R . Rodriguez,
linux-kernel, David S . Miller, Olivier Brunel
In-Reply-To: <20181020173957.31239-1-jjk@jjacky.com>
Signed-off-by: Olivier Brunel <jjk@jjacky.com>
---
net/bpfilter/bpfilter_kern.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/bpfilter/bpfilter_kern.c b/net/bpfilter/bpfilter_kern.c
index 94e88f510..7acfc8308 100644
--- a/net/bpfilter/bpfilter_kern.c
+++ b/net/bpfilter/bpfilter_kern.c
@@ -92,6 +92,7 @@ static int __init load_umh(void)
int err;
/* fork usermode process */
+ info.cmdline = "bpfilter_umh";
err = fork_usermode_blob(&bpfilter_umh_start,
&bpfilter_umh_end - &bpfilter_umh_start,
&info);
--
2.19.0
^ permalink raw reply related
* [PATCH 1/2] umh: Add command line to user mode helpers
From: Olivier Brunel @ 2018-10-20 17:39 UTC (permalink / raw)
To: Network Development
Cc: Alexei Starovoitov, Daniel Borkmann, Luis R . Rodriguez,
linux-kernel, David S . Miller, Olivier Brunel
In-Reply-To: <20181020173957.31239-1-jjk@jjacky.com>
User mode helpers were spawned without a command line, and because
an empty command line is used by many tools to identify processes as
kernel threads, this could cause some issues.
Notably during killing spree on shutdown, since such helper would then
be skipped (i.e. not killed) which would result in the process remaining
alive, and thus preventing unmouting of the rootfs (as experienced with
the bpfilter umh).
Fixes: 449325b52b7a ("umh: introduce fork_usermode_blob() helper")
Signed-off-by: Olivier Brunel <jjk@jjacky.com>
---
include/linux/umh.h | 1 +
kernel/umh.c | 16 ++++++++++++++--
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/include/linux/umh.h b/include/linux/umh.h
index 5c812acbb..235f51b62 100644
--- a/include/linux/umh.h
+++ b/include/linux/umh.h
@@ -44,6 +44,7 @@ struct subprocess_info *call_usermodehelper_setup_file(struct file *file,
int (*init)(struct subprocess_info *info, struct cred *new),
void (*cleanup)(struct subprocess_info *), void *data);
struct umh_info {
+ const char *cmdline;
struct file *pipe_to_umh;
struct file *pipe_from_umh;
pid_t pid;
diff --git a/kernel/umh.c b/kernel/umh.c
index c44985894..0baa672e0 100644
--- a/kernel/umh.c
+++ b/kernel/umh.c
@@ -405,11 +405,19 @@ struct subprocess_info *call_usermodehelper_setup_file(struct file *file,
void (*cleanup)(struct subprocess_info *info), void *data)
{
struct subprocess_info *sub_info;
+ struct umh_info *info = data;
+ const char *cmdline = (info->cmdline) ? info->cmdline : "usermodehelper";
sub_info = kzalloc(sizeof(struct subprocess_info), GFP_KERNEL);
if (!sub_info)
return NULL;
+ sub_info->argv = argv_split(GFP_KERNEL, cmdline, NULL);
+ if (!sub_info->argv) {
+ kfree(sub_info);
+ return NULL;
+ }
+
INIT_WORK(&sub_info->work, call_usermodehelper_exec_work);
sub_info->path = "none";
sub_info->file = file;
@@ -458,10 +466,11 @@ static int umh_pipe_setup(struct subprocess_info *info, struct cred *new)
return 0;
}
-static void umh_save_pid(struct subprocess_info *info)
+static void umh_clean_and_save_pid(struct subprocess_info *info)
{
struct umh_info *umh_info = info->data;
+ argv_free(info->argv);
umh_info->pid = info->pid;
}
@@ -471,6 +480,9 @@ static void umh_save_pid(struct subprocess_info *info)
* @len: length of the blob
* @info: information about usermode process (shouldn't be NULL)
*
+ * If info->cmdline is set it will be used as command line for the
+ * user process, else "usermodehelper" is used.
+ *
* Returns either negative error or zero which indicates success
* in executing a blob of bytes as a usermode process. In such
* case 'struct umh_info *info' is populated with two pipes
@@ -500,7 +512,7 @@ int fork_usermode_blob(void *data, size_t len, struct umh_info *info)
err = -ENOMEM;
sub_info = call_usermodehelper_setup_file(file, umh_pipe_setup,
- umh_save_pid, info);
+ umh_clean_and_save_pid, info);
if (!sub_info)
goto out;
--
2.19.0
^ permalink raw reply related
* [PATCH 0/2] Re: bpfilter causes a leftover kernel process
From: Olivier Brunel @ 2018-10-20 17:39 UTC (permalink / raw)
To: Network Development
Cc: Alexei Starovoitov, Daniel Borkmann, Luis R . Rodriguez,
linux-kernel, David S . Miller, Olivier Brunel
In-Reply-To: <CAADnVQ+xFfwV67BRXD0epoTwKz54W3mcYHXVWBd1iWb94p+zpg@mail.gmail.com>
On Tue, 16 Oct 2018 16:38:56 +0000
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> On Wed, Sep 5, 2018 at 5:05 PM Olivier Brunel <jjk@jjacky.com> wrote:
> >
> > You'll see in the end that systemd complains that it can't
> > unmount /oldroot (EBUSY), aka the root fs; and that's because of the
> > bpfilter helper, which wasn't killed because it's seen as a kernel
> > thread due to its empty command line and therefore not signaled.
>
> thanks for tracking it down.
> can somebody send a patch to give bpfilter non-empty cmdline?
> I think that would be a better fix than tweaking all pid1s.
So I'm not a kernel dev and this would be my first atttempt at a kernel patch,
but I did have a look and came up with the following patch(es) to fix this.
Hopefully I did things right.
It adds a default command line ("usermodehelper") to such processes, so any &
all such helpers will be seen as user process and not kernel threads, but
there's also the possibility to specify a command line to use, here
"bpfilter_umh"
Cheers,
Olivier Brunel (2):
umh: Add command line to user mode helpers
net: bpfilter: Set user mode helper's command line
include/linux/umh.h | 1 +
kernel/umh.c | 16 ++++++++++++++--
net/bpfilter/bpfilter_kern.c | 1 +
3 files changed, 16 insertions(+), 2 deletions(-)
--
2.19.0
^ permalink raw reply
* Re: [PATCH] net: ethernet: fec: Add missing SPEED_
From: Andrew Lunn @ 2018-10-20 15:39 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Florian Fainelli, LABBE Corentin, davem, fugang.duan,
linux-kernel, netdev
In-Reply-To: <f1933c0a-d124-a7ac-afab-2fa8f88907ab@gmail.com>
> >> I have patched by adding:
> >> phy_remove_link_mode(phy_dev, ETHTOOL_LINK_MODE_Asym_Pause_BIT);
>
> Instead of programmatically removing the feature bit it should be
> possible to do this in the PHY driver configuration. See also
> this part of phy_probe().
>
> if (phydrv->features & (SUPPORTED_Pause | SUPPORTED_Asym_Pause)) {
> phydev->supported &= ~(SUPPORTED_Pause | SUPPORTED_Asym_Pause);
> phydev->supported |= phydrv->features &
> (SUPPORTED_Pause | SUPPORTED_Asym_Pause);
> } else {
> phydev->supported |= SUPPORTED_Pause | SUPPORTED_Asym_Pause;
> }
Sorry for the late reply. Been on vacation.
I need to check the datasheet, but it seems like the KSZ9021 does not
support asym pause. Using the above code is the correct way to solve
this problem. Look at the bcm63xx.c:bcm63xx_config_init() which does
this.
I will cook up a patch.
Andrew
^ permalink raw reply
* [PATCH] net: socket: fix a missing-check bug
From: Wenwen Wang @ 2018-10-20 15:58 UTC (permalink / raw)
To: Wenwen Wang
Cc: Kangjie Lu, David S. Miller, open list:NETWORKING [GENERAL],
open list
In ethtool_ioctl(), the ioctl command is firstly obtained from the
user-space buffer 'compat_rxnfc' through get_user() and saved to 'ethcmd'.
Then, 'ethcmd' is checked to see whether it is necessary to pre-process the
ethool structure, because the structure ethtool_rxnfc is defined with
padding, as mentioned in the comment. If yes, a user-space buffer 'rxnfc'
is allocated through compat_alloc_user_space() and then the data in the
original buffer 'compat_rxnfc' is copied to 'rxnfc' through copy_in_user(),
including the ioctl command. It is worth noting that after this copy, there
is no check enforced on the copied ioctl command. That means it is possible
that 'rxnfc->cmd' is different from 'ethcmd', because a malicious user can
race to modify the ioctl command in 'compat_rxnfc' between these two
copies. Eventually, the ioctl command in 'rxnfc' will be used in
dev_ethtool(). This can cause undefined behavior of the kernel and
introduce potential security risk.
This patch avoids the above issue by rewriting 'rxnfc->cmd' using 'ethcmd'
after copy_in_user().
Signed-off-by: Wenwen Wang <wang6495@umn.edu>
---
net/socket.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/socket.c b/net/socket.c
index 01f3f8f..c5f969c 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2879,6 +2879,8 @@ static int ethtool_ioctl(struct net *net, struct compat_ifreq __user *ifr32)
copy_in_user(&rxnfc->rule_cnt, &compat_rxnfc->rule_cnt,
sizeof(rxnfc->rule_cnt)))
return -EFAULT;
+
+ rxnfc->cmd = ethcmd;
}
ret = dev_ioctl(net, SIOCETHTOOL, &ifr, NULL);
--
2.7.4
^ permalink raw reply related
* [GIT] Networking
From: David Miller @ 2018-10-20 22:47 UTC (permalink / raw)
To: gregkh; +Cc: akpm, netdev, linux-kernel
A few straggler bug fixes:
1) Fix indexing of multi-pass dumps of ipv6 addresses, from David
Ahern.
2) Revert RCU locking change for bonding netpoll, causes worse problems
than it solves.
3) pskb_trim_rcsum_slow() doesn't handle odd trim offsets, resulting in
erroneous bad hw checksum triggers with CHECKSUM_COMPLETE devices.
From Dimitris Michailidis.
Please pull, thanks a lot!
The following changes since commit 91b15613ce7fb3e724ca0d433eef8e6bf15322af:
Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2018-10-19 09:16:20 +0200)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net
for you to fetch changes up to 4ba4c566ba8448a05e6257e0b98a21f1a0d55315:
net/ipv6: Fix index counter for unicast addresses in in6_dump_addrs (2018-10-20 15:43:14 -0700)
----------------------------------------------------------------
David Ahern (1):
net/ipv6: Fix index counter for unicast addresses in in6_dump_addrs
David S. Miller (1):
Revert "bond: take rcu lock in netpoll_send_skb_on_dev"
Dimitris Michailidis (1):
net: fix pskb_trim_rcsum_slow() with odd trim offset
net/core/netpoll.c | 2 --
net/core/skbuff.c | 5 +++--
net/ipv6/addrconf.c | 6 ++++--
3 files changed, 7 insertions(+), 6 deletions(-)
^ permalink raw reply
* Re: [PATCH net-next v8 28/28] net: WireGuard secure network tunnel
From: Andrew Lunn @ 2018-10-20 22:47 UTC (permalink / raw)
To: Jason A. Donenfeld; +Cc: linux-kernel, netdev, linux-crypto, davem, gregkh
In-Reply-To: <20181018145712.7538-29-Jason@zx2c4.com>
> +#define choose_node(parent, key) \
> + parent->bit[(key[parent->bit_at_a] >> parent->bit_at_b) & 1]
Hi Jason
This should be a function, not a macro.
> +
> +static void node_free_rcu(struct rcu_head *rcu)
> +{
> + kfree(container_of(rcu, struct allowedips_node, rcu));
> +}
> +
> +#define push_rcu(stack, p, len) ({ \
> + if (rcu_access_pointer(p)) { \
> + WARN_ON(IS_ENABLED(DEBUG) && (len) >= 128); \
> + stack[(len)++] = rcu_dereference_raw(p); \
> + } \
> + true; \
> + })
This also looks like it could be a function.
> +static void root_free_rcu(struct rcu_head *rcu)
> +{
> + struct allowedips_node *node, *stack[128] = {
> + container_of(rcu, struct allowedips_node, rcu) };
> + unsigned int len = 1;
> +
> + while (len > 0 && (node = stack[--len]) &&
> + push_rcu(stack, node->bit[0], len) &&
> + push_rcu(stack, node->bit[1], len))
> + kfree(node);
> +}
> +
> +#define ref(p) rcu_access_pointer(p)
> +#define deref(p) rcu_dereference_protected(*(p), lockdep_is_held(lock))
Macros should be uppercase, or better still, functions.
> +#define push(p) ({ \
> + WARN_ON(IS_ENABLED(DEBUG) && len >= 128); \
> + stack[len++] = p; \
> + })
This one definitely should be upper case, to warn readers it has
unexpected side effects.
> +
> +static void walk_remove_by_peer(struct allowedips_node __rcu **top,
> + struct wg_peer *peer, struct mutex *lock)
> +{
> + struct allowedips_node __rcu **stack[128], **nptr;
> + struct allowedips_node *node, *prev;
> + unsigned int len;
> +
> + if (unlikely(!peer || !ref(*top)))
> + return;
> +
> + for (prev = NULL, len = 0, push(top); len > 0; prev = node) {
> + nptr = stack[len - 1];
> + node = deref(nptr);
> + if (!node) {
> + --len;
> + continue;
> + }
> + if (!prev || ref(prev->bit[0]) == node ||
> + ref(prev->bit[1]) == node) {
> + if (ref(node->bit[0]))
> + push(&node->bit[0]);
> + else if (ref(node->bit[1]))
> + push(&node->bit[1]);
> + } else if (ref(node->bit[0]) == prev) {
> + if (ref(node->bit[1]))
> + push(&node->bit[1]);
> + } else {
> + if (rcu_dereference_protected(node->peer,
> + lockdep_is_held(lock)) == peer) {
> + RCU_INIT_POINTER(node->peer, NULL);
> + if (!node->bit[0] || !node->bit[1]) {
> + rcu_assign_pointer(*nptr,
> + deref(&node->bit[!ref(node->bit[0])]));
> + call_rcu_bh(&node->rcu, node_free_rcu);
> + node = deref(nptr);
> + }
> + }
> + --len;
> + }
> + }
> +}
> +
> +#undef ref
> +#undef deref
> +#undef push
> +
> +static __always_inline unsigned int fls128(u64 a, u64 b)
> +{
> + return a ? fls64(a) + 64U : fls64(b);
> +}
Does the compiler actually get this wrong? Not inline it? There was
an interesting LWN post about this recently:
https://lwn.net/Articles/767884/
But in general, inline of any form should be avoided in .c files.
> +
> +static __always_inline u8 common_bits(const struct allowedips_node *node,
> + const u8 *key, u8 bits)
> +{
> + if (bits == 32)
> + return 32U - fls(*(const u32 *)node->bits ^ *(const u32 *)key);
> + else if (bits == 128)
> + return 128U - fls128(
> + *(const u64 *)&node->bits[0] ^ *(const u64 *)&key[0],
> + *(const u64 *)&node->bits[8] ^ *(const u64 *)&key[8]);
> + return 0;
> +}
> +
> +/* This could be much faster if it actually just compared the common bits
> + * properly, by precomputing a mask bswap(~0 << (32 - cidr)), and the rest, but
> + * it turns out that common_bits is already super fast on modern processors,
> + * even taking into account the unfortunate bswap. So, we just inline it like
> + * this instead.
> + */
> +#define prefix_matches(node, key, bits) \
> + (common_bits(node, key, bits) >= (node)->cidr)
Could be a function.
> +
> +static __always_inline struct allowedips_node *
> +find_node(struct allowedips_node *trie, u8 bits, const u8 *key)
> +{
> + struct allowedips_node *node = trie, *found = NULL;
> +
> + while (node && prefix_matches(node, key, bits)) {
> + if (rcu_access_pointer(node->peer))
> + found = node;
> + if (node->cidr == bits)
> + break;
> + node = rcu_dereference_bh(choose_node(node, key));
> + }
> + return found;
> +}
> +
> +/* Returns a strong reference to a peer */
> +static __always_inline struct wg_peer *
> +lookup(struct allowedips_node __rcu *root, u8 bits, const void *be_ip)
> +{
> + u8 ip[16] __aligned(__alignof(u64));
You virtually never see aligned stack variables. This needs some sort
of comment.
> + struct allowedips_node *node;
> + struct wg_peer *peer = NULL;
> +
> + swap_endian(ip, be_ip, bits);
> +
> + rcu_read_lock_bh();
> +retry:
> + node = find_node(rcu_dereference_bh(root), bits, ip);
> + if (node) {
> + peer = wg_peer_get_maybe_zero(rcu_dereference_bh(node->peer));
> + if (!peer)
> + goto retry;
> + }
> + rcu_read_unlock_bh();
> + return peer;
> +}
> +
> +__attribute__((nonnull(1))) static bool
> +node_placement(struct allowedips_node __rcu *trie, const u8 *key, u8 cidr,
> + u8 bits, struct allowedips_node **rnode, struct mutex *lock)
> +{
> + struct allowedips_node *node = rcu_dereference_protected(trie,
> + lockdep_is_held(lock));
> + struct allowedips_node *parent = NULL;
> + bool exact = false;
Should there be a WARN_ON(!key) here, since the attribute will only
detect problems at compile time, and maybe some runtime cases will get
passed it?
> +
> + while (node && node->cidr <= cidr && prefix_matches(node, key, bits)) {
> + parent = node;
> + if (parent->cidr == cidr) {
> + exact = true;
> + break;
> + }
> + node = rcu_dereference_protected(choose_node(parent, key),
> + lockdep_is_held(lock));
> + }
> + *rnode = parent;
> + return exact;
> +}
> +void wg_cookie_message_consume(struct message_handshake_cookie *src,
> + struct wg_device *wg)
> +{
> + struct wg_peer *peer = NULL;
> + u8 cookie[COOKIE_LEN];
> + bool ret;
> +
> + if (unlikely(!wg_index_hashtable_lookup(&wg->index_hashtable,
> + INDEX_HASHTABLE_HANDSHAKE |
> + INDEX_HASHTABLE_KEYPAIR,
> + src->receiver_index, &peer)))
> + return;
> +
> + down_read(&peer->latest_cookie.lock);
> + if (unlikely(!peer->latest_cookie.have_sent_mac1)) {
> + up_read(&peer->latest_cookie.lock);
> + goto out;
> + }
> + ret = xchacha20poly1305_decrypt(
> + cookie, src->encrypted_cookie, sizeof(src->encrypted_cookie),
> + peer->latest_cookie.last_mac1_sent, COOKIE_LEN, src->nonce,
> + peer->latest_cookie.cookie_decryption_key);
> + up_read(&peer->latest_cookie.lock);
> +
> + if (ret) {
> + down_write(&peer->latest_cookie.lock);
> + memcpy(peer->latest_cookie.cookie, cookie, COOKIE_LEN);
> + peer->latest_cookie.birthdate = ktime_get_boot_fast_ns();
> + peer->latest_cookie.is_valid = true;
> + peer->latest_cookie.have_sent_mac1 = false;
> + up_write(&peer->latest_cookie.lock);
> + } else {
> + net_dbg_ratelimited("%s: Could not decrypt invalid cookie response\n",
> + wg->dev->name);
It might be worth adding a netdev_dbg_ratelimited(), which takes a
netdev as its first parameter, just line netdev_dbg().
> +static int wg_open(struct net_device *dev)
> +{
> + struct in_device *dev_v4 = __in_dev_get_rtnl(dev);
> + struct inet6_dev *dev_v6 = __in6_dev_get(dev);
> + struct wg_device *wg = netdev_priv(dev);
> + struct wg_peer *peer;
> + int ret;
> +
> + if (dev_v4) {
> + /* At some point we might put this check near the ip_rt_send_
> + * redirect call of ip_forward in net/ipv4/ip_forward.c, similar
> + * to the current secpath check.
> + */
> + IN_DEV_CONF_SET(dev_v4, SEND_REDIRECTS, false);
> + IPV4_DEVCONF_ALL(dev_net(dev), SEND_REDIRECTS) = false;
> + }
> + if (dev_v6)
> + dev_v6->cnf.addr_gen_mode = IN6_ADDR_GEN_MODE_NONE;
> +
> + ret = wg_socket_init(wg, wg->incoming_port);
> + if (ret < 0)
> + return ret;
> + mutex_lock(&wg->device_update_lock);
> + list_for_each_entry(peer, &wg->peer_list, peer_list) {
> + wg_packet_send_staged_packets(peer);
> + if (peer->persistent_keepalive_interval)
> + wg_packet_send_keepalive(peer);
> + }
> + mutex_unlock(&wg->device_update_lock);
> + return 0;
> +}
> +
> +#if defined(CONFIG_PM_SLEEP) && !defined(CONFIG_ANDROID)
I don't see any other code which uses this combination. Why is this
needed?
> +static netdev_tx_t wg_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> + struct wg_device *wg = netdev_priv(dev);
> + struct sk_buff_head packets;
> + struct wg_peer *peer;
> + struct sk_buff *next;
> + sa_family_t family;
> + u32 mtu;
> + int ret;
> +
> + if (unlikely(wg_skb_examine_untrusted_ip_hdr(skb) != skb->protocol)) {
> + ret = -EPROTONOSUPPORT;
> + net_dbg_ratelimited("%s: Invalid IP packet\n", dev->name);
> + goto err;
> + }
> +
> + peer = wg_allowedips_lookup_dst(&wg->peer_allowedips, skb);
> + if (unlikely(!peer)) {
> + ret = -ENOKEY;
> + if (skb->protocol == htons(ETH_P_IP))
> + net_dbg_ratelimited("%s: No peer has allowed IPs matching %pI4\n",
> + dev->name, &ip_hdr(skb)->daddr);
> + else if (skb->protocol == htons(ETH_P_IPV6))
> + net_dbg_ratelimited("%s: No peer has allowed IPs matching %pI6\n",
> + dev->name, &ipv6_hdr(skb)->daddr);
> + goto err;
> + }
> +
> + family = READ_ONCE(peer->endpoint.addr.sa_family);
> + if (unlikely(family != AF_INET && family != AF_INET6)) {
> + ret = -EDESTADDRREQ;
> + net_dbg_ratelimited("%s: No valid endpoint has been configured or discovered for peer %llu\n",
> + dev->name, peer->internal_id);
> + goto err_peer;
> + }
> +
> + mtu = skb_dst(skb) ? dst_mtu(skb_dst(skb)) : dev->mtu;
> +
> + __skb_queue_head_init(&packets);
> + if (!skb_is_gso(skb)) {
> + skb->next = NULL;
> + } else {
> + struct sk_buff *segs = skb_gso_segment(skb, 0);
> +
> + if (unlikely(IS_ERR(segs))) {
> + ret = PTR_ERR(segs);
> + goto err_peer;
> + }
> + dev_kfree_skb(skb);
> + skb = segs;
> + }
> + do {
> + next = skb->next;
> + skb->next = skb->prev = NULL;
> +
> + skb = skb_share_check(skb, GFP_ATOMIC);
> + if (unlikely(!skb))
> + continue;
> +
> + /* We only need to keep the original dst around for icmp,
> + * so at this point we're in a position to drop it.
> + */
> + skb_dst_drop(skb);
> +
> + PACKET_CB(skb)->mtu = mtu;
> +
> + __skb_queue_tail(&packets, skb);
> + } while ((skb = next) != NULL);
> +
> + spin_lock_bh(&peer->staged_packet_queue.lock);
> + /* If the queue is getting too big, we start removing the oldest packets
> + * until it's small again. We do this before adding the new packet, so
> + * we don't remove GSO segments that are in excess.
> + */
> + while (skb_queue_len(&peer->staged_packet_queue) > MAX_STAGED_PACKETS)
> + dev_kfree_skb(__skb_dequeue(&peer->staged_packet_queue));
It would be good to have some stats counters in here. Maybe the
standard interface statistics will cover it, otherwise ethtool -S.
You should also get this code looked at by some of the queueing
people. Rather than discarding, you might want to be applying back
pressure to slow down the sender application?
> +static void kdf(u8 *first_dst, u8 *second_dst, u8 *third_dst, const u8 *data,
> + size_t first_len, size_t second_len, size_t third_len,
> + size_t data_len, const u8 chaining_key[NOISE_HASH_LEN])
> +{
> + u8 output[BLAKE2S_HASH_SIZE + 1];
> + u8 secret[BLAKE2S_HASH_SIZE];
> +
> + WARN_ON(IS_ENABLED(DEBUG) &&
> + (first_len > BLAKE2S_HASH_SIZE ||
> + second_len > BLAKE2S_HASH_SIZE ||
> + third_len > BLAKE2S_HASH_SIZE ||
> + ((second_len || second_dst || third_len || third_dst) &&
> + (!first_len || !first_dst)) ||
> + ((third_len || third_dst) && (!second_len || !second_dst))));
Maybe split this up into a number of WARN_ON()s. At the moment, if it
fires, you have little idea why, there are so many comparisons. Also,
is this on the hot path? I guess not, since this is keys, not
packets. Do you need to care about DEBUG here?
Andrew
^ 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