* Re: [PATCH] net: ethernet: fec: Add missing SPEED_
From: Heiner Kallweit @ 2018-10-20 15:51 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, LABBE Corentin, davem, fugang.duan,
linux-kernel, netdev
In-Reply-To: <20181020153922.GC1596@lunn.ch>
On 20.10.2018 17:39, Andrew Lunn wrote:
>>>> 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 dare to dispute here ;) Above code snippet from phy_probe() will
(try to) set also SUPPORTED_Asym_Pause, because phydrv->features
doesn't include any of the two pause flags.
The statement in bcm63xx_config_init you refer to seems to be a
no-op to me therefore.
I'd say the correct way is to change the PHY config like this:
.features = PHY_BASIC_FEATURES | SUPPORTED_Pause;
It's exactly the use case the code snippet above covers.
I think the bcm63xx driver would need to be changed.
> I will cook up a patch.
>
> Andrew
>
^ permalink raw reply
* [bpf-next v2 3/3] bpf: test_sockmap add options to use msg_push_data
From: John Fastabend @ 2018-10-20 2:56 UTC (permalink / raw)
To: ast, daniel; +Cc: john.fastabend, netdev
In-Reply-To: <1540004211-7647-1-git-send-email-john.fastabend@gmail.com>
Add options to run msg_push_data, this patch creates two more flags
in test_sockmap that can be used to specify the offset and length
of bytes to be added. The new options are --txmsg_start_push to
specify where bytes should be inserted and --txmsg_end_push to
specify how many bytes. This is analagous to the options that are
used to pull data, --txmsg_start and --txmsg_end.
In addition to adding the options tests are added to the test
suit to run the tests similar to what was done for msg_pull_data.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
tools/testing/selftests/bpf/test_sockmap.c | 58 ++++++++++++++-
tools/testing/selftests/bpf/test_sockmap_kern.h | 97 +++++++++++++++++++------
2 files changed, 129 insertions(+), 26 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c
index cbd1c0b..622ade0 100644
--- a/tools/testing/selftests/bpf/test_sockmap.c
+++ b/tools/testing/selftests/bpf/test_sockmap.c
@@ -77,6 +77,8 @@
int txmsg_cork;
int txmsg_start;
int txmsg_end;
+int txmsg_start_push;
+int txmsg_end_push;
int txmsg_ingress;
int txmsg_skb;
int ktls;
@@ -100,6 +102,8 @@
{"txmsg_cork", required_argument, NULL, 'k'},
{"txmsg_start", required_argument, NULL, 's'},
{"txmsg_end", required_argument, NULL, 'e'},
+ {"txmsg_start_push", required_argument, NULL, 'p'},
+ {"txmsg_end_push", required_argument, NULL, 'q'},
{"txmsg_ingress", no_argument, &txmsg_ingress, 1 },
{"txmsg_skb", no_argument, &txmsg_skb, 1 },
{"ktls", no_argument, &ktls, 1 },
@@ -903,6 +907,30 @@ static int run_options(struct sockmap_options *options, int cg_fd, int test)
}
}
+ if (txmsg_start_push) {
+ i = 2;
+ err = bpf_map_update_elem(map_fd[5],
+ &i, &txmsg_start_push, BPF_ANY);
+ if (err) {
+ fprintf(stderr,
+ "ERROR: bpf_map_update_elem (txmsg_start_push): %d (%s)\n",
+ err, strerror(errno));
+ goto out;
+ }
+ }
+
+ if (txmsg_end_push) {
+ i = 3;
+ err = bpf_map_update_elem(map_fd[5],
+ &i, &txmsg_end_push, BPF_ANY);
+ if (err) {
+ fprintf(stderr,
+ "ERROR: bpf_map_update_elem %i@%i (txmsg_end_push): %d (%s)\n",
+ txmsg_end_push, i, err, strerror(errno));
+ goto out;
+ }
+ }
+
if (txmsg_ingress) {
int in = BPF_F_INGRESS;
@@ -1235,6 +1263,8 @@ static int test_mixed(int cgrp)
txmsg_pass = txmsg_noisy = txmsg_redir_noisy = txmsg_drop = 0;
txmsg_apply = txmsg_cork = 0;
txmsg_start = txmsg_end = 0;
+ txmsg_start_push = txmsg_end_push = 0;
+
/* Test small and large iov_count values with pass/redir/apply/cork */
txmsg_pass = 1;
txmsg_redir = 0;
@@ -1351,6 +1381,8 @@ static int test_start_end(int cgrp)
/* Test basic start/end with lots of iov_count and iov_lengths */
txmsg_start = 1;
txmsg_end = 2;
+ txmsg_start_push = 1;
+ txmsg_end_push = 2;
err = test_txmsg(cgrp);
if (err)
goto out;
@@ -1364,6 +1396,8 @@ static int test_start_end(int cgrp)
for (i = 99; i <= 1600; i += 500) {
txmsg_start = 0;
txmsg_end = i;
+ txmsg_start_push = 0;
+ txmsg_end_push = i;
err = test_exec(cgrp, &opt);
if (err)
goto out;
@@ -1373,6 +1407,8 @@ static int test_start_end(int cgrp)
for (i = 199; i <= 1600; i += 500) {
txmsg_start = 100;
txmsg_end = i;
+ txmsg_start_push = 100;
+ txmsg_end_push = i;
err = test_exec(cgrp, &opt);
if (err)
goto out;
@@ -1381,6 +1417,8 @@ static int test_start_end(int cgrp)
/* Test start/end with cork pulling last sg entry */
txmsg_start = 1500;
txmsg_end = 1600;
+ txmsg_start_push = 1500;
+ txmsg_end_push = 1600;
err = test_exec(cgrp, &opt);
if (err)
goto out;
@@ -1388,6 +1426,8 @@ static int test_start_end(int cgrp)
/* Test start/end pull of single byte in last page */
txmsg_start = 1111;
txmsg_end = 1112;
+ txmsg_start_push = 1111;
+ txmsg_end_push = 1112;
err = test_exec(cgrp, &opt);
if (err)
goto out;
@@ -1395,6 +1435,8 @@ static int test_start_end(int cgrp)
/* Test start/end with end < start */
txmsg_start = 1111;
txmsg_end = 0;
+ txmsg_start_push = 1111;
+ txmsg_end_push = 0;
err = test_exec(cgrp, &opt);
if (err)
goto out;
@@ -1402,6 +1444,8 @@ static int test_start_end(int cgrp)
/* Test start/end with end > data */
txmsg_start = 0;
txmsg_end = 1601;
+ txmsg_start_push = 0;
+ txmsg_end_push = 1601;
err = test_exec(cgrp, &opt);
if (err)
goto out;
@@ -1409,6 +1453,8 @@ static int test_start_end(int cgrp)
/* Test start/end with start > data */
txmsg_start = 1601;
txmsg_end = 1600;
+ txmsg_start_push = 1601;
+ txmsg_end_push = 1600;
err = test_exec(cgrp, &opt);
out:
@@ -1424,7 +1470,7 @@ static int test_start_end(int cgrp)
"sock_map_redir",
"sock_apply_bytes",
"sock_cork_bytes",
- "sock_pull_bytes",
+ "sock_bytes",
"sock_redir_flags",
"sock_skb_opts",
};
@@ -1531,7 +1577,7 @@ static int __test_suite(int cg_fd, char *bpf_file)
}
/* Tests basic commands and APIs with range of iov values */
- txmsg_start = txmsg_end = 0;
+ txmsg_start = txmsg_end = txmsg_start_push = txmsg_end_push = 0;
err = test_txmsg(cg_fd);
if (err)
goto out;
@@ -1580,7 +1626,7 @@ int main(int argc, char **argv)
if (argc < 2)
return test_suite(-1);
- while ((opt = getopt_long(argc, argv, ":dhvc:r:i:l:t:",
+ while ((opt = getopt_long(argc, argv, ":dhvc:r:i:l:t:p:q:",
long_options, &longindex)) != -1) {
switch (opt) {
case 's':
@@ -1589,6 +1635,12 @@ int main(int argc, char **argv)
case 'e':
txmsg_end = atoi(optarg);
break;
+ case 'p':
+ txmsg_start_push = atoi(optarg);
+ break;
+ case 'q':
+ txmsg_end_push = atoi(optarg);
+ break;
case 'a':
txmsg_apply = atoi(optarg);
break;
diff --git a/tools/testing/selftests/bpf/test_sockmap_kern.h b/tools/testing/selftests/bpf/test_sockmap_kern.h
index 8e8e417..14b8bba 100644
--- a/tools/testing/selftests/bpf/test_sockmap_kern.h
+++ b/tools/testing/selftests/bpf/test_sockmap_kern.h
@@ -70,11 +70,11 @@ struct bpf_map_def SEC("maps") sock_cork_bytes = {
.max_entries = 1
};
-struct bpf_map_def SEC("maps") sock_pull_bytes = {
+struct bpf_map_def SEC("maps") sock_bytes = {
.type = BPF_MAP_TYPE_ARRAY,
.key_size = sizeof(int),
.value_size = sizeof(int),
- .max_entries = 2
+ .max_entries = 4
};
struct bpf_map_def SEC("maps") sock_redir_flags = {
@@ -181,8 +181,8 @@ int bpf_sockmap(struct bpf_sock_ops *skops)
SEC("sk_msg1")
int bpf_prog4(struct sk_msg_md *msg)
{
- int *bytes, zero = 0, one = 1;
- int *start, *end;
+ int *bytes, zero = 0, one = 1, two = 2, three = 3;
+ int *start, *end, *start_push, *end_push;
bytes = bpf_map_lookup_elem(&sock_apply_bytes, &zero);
if (bytes)
@@ -190,18 +190,24 @@ int bpf_prog4(struct sk_msg_md *msg)
bytes = bpf_map_lookup_elem(&sock_cork_bytes, &zero);
if (bytes)
bpf_msg_cork_bytes(msg, *bytes);
- start = bpf_map_lookup_elem(&sock_pull_bytes, &zero);
- end = bpf_map_lookup_elem(&sock_pull_bytes, &one);
+ start = bpf_map_lookup_elem(&sock_bytes, &zero);
+ end = bpf_map_lookup_elem(&sock_bytes, &one);
if (start && end)
bpf_msg_pull_data(msg, *start, *end, 0);
+ start_push = bpf_map_lookup_elem(&sock_bytes, &two);
+ end_push = bpf_map_lookup_elem(&sock_bytes, &three);
+ if (start_push && end_push)
+ bpf_msg_push_data(msg, *start_push, *end_push, 0);
return SK_PASS;
}
SEC("sk_msg2")
int bpf_prog5(struct sk_msg_md *msg)
{
- int err1 = -1, err2 = -1, zero = 0, one = 1;
- int *bytes, *start, *end, len1, len2;
+ int zero = 0, one = 1, two = 2, three = 3;
+ int *start, *end, *start_push, *end_push;
+ int *bytes, len1, len2 = 0, len3;
+ int err1 = -1, err2 = -1;
bytes = bpf_map_lookup_elem(&sock_apply_bytes, &zero);
if (bytes)
@@ -210,8 +216,8 @@ int bpf_prog5(struct sk_msg_md *msg)
if (bytes)
err2 = bpf_msg_cork_bytes(msg, *bytes);
len1 = (__u64)msg->data_end - (__u64)msg->data;
- start = bpf_map_lookup_elem(&sock_pull_bytes, &zero);
- end = bpf_map_lookup_elem(&sock_pull_bytes, &one);
+ start = bpf_map_lookup_elem(&sock_bytes, &zero);
+ end = bpf_map_lookup_elem(&sock_bytes, &one);
if (start && end) {
int err;
@@ -225,6 +231,23 @@ int bpf_prog5(struct sk_msg_md *msg)
bpf_printk("sk_msg2: length update %i->%i\n",
len1, len2);
}
+
+ start_push = bpf_map_lookup_elem(&sock_bytes, &two);
+ end_push = bpf_map_lookup_elem(&sock_bytes, &three);
+ if (start_push && end_push) {
+ int err;
+
+ bpf_printk("sk_msg2: push(%i:%i)\n",
+ start_push ? *start_push : 0,
+ end_push ? *end_push : 0);
+ err = bpf_msg_push_data(msg, *start_push, *end_push, 0);
+ if (err)
+ bpf_printk("sk_msg2: push_data err %i\n", err);
+ len3 = (__u64)msg->data_end - (__u64)msg->data;
+ bpf_printk("sk_msg2: length push_update %i->%i\n",
+ len2 ? len2 : len1, len3);
+ }
+
bpf_printk("sk_msg2: data length %i err1 %i err2 %i\n",
len1, err1, err2);
return SK_PASS;
@@ -233,8 +256,8 @@ int bpf_prog5(struct sk_msg_md *msg)
SEC("sk_msg3")
int bpf_prog6(struct sk_msg_md *msg)
{
- int *bytes, zero = 0, one = 1, key = 0;
- int *start, *end, *f;
+ int *bytes, *start, *end, *start_push, *end_push, *f;
+ int zero = 0, one = 1, two = 2, three = 3, key = 0;
__u64 flags = 0;
bytes = bpf_map_lookup_elem(&sock_apply_bytes, &zero);
@@ -243,10 +266,17 @@ int bpf_prog6(struct sk_msg_md *msg)
bytes = bpf_map_lookup_elem(&sock_cork_bytes, &zero);
if (bytes)
bpf_msg_cork_bytes(msg, *bytes);
- start = bpf_map_lookup_elem(&sock_pull_bytes, &zero);
- end = bpf_map_lookup_elem(&sock_pull_bytes, &one);
+
+ start = bpf_map_lookup_elem(&sock_bytes, &zero);
+ end = bpf_map_lookup_elem(&sock_bytes, &one);
if (start && end)
bpf_msg_pull_data(msg, *start, *end, 0);
+
+ start_push = bpf_map_lookup_elem(&sock_bytes, &two);
+ end_push = bpf_map_lookup_elem(&sock_bytes, &three);
+ if (start_push && end_push)
+ bpf_msg_push_data(msg, *start_push, *end_push, 0);
+
f = bpf_map_lookup_elem(&sock_redir_flags, &zero);
if (f && *f) {
key = 2;
@@ -262,8 +292,9 @@ int bpf_prog6(struct sk_msg_md *msg)
SEC("sk_msg4")
int bpf_prog7(struct sk_msg_md *msg)
{
- int err1 = 0, err2 = 0, zero = 0, one = 1, key = 0;
- int *f, *bytes, *start, *end, len1, len2;
+ int zero = 0, one = 1, two = 2, three = 3, len1, len2 = 0, len3;
+ int *bytes, *start, *end, *start_push, *end_push, *f;
+ int err1 = 0, err2 = 0, key = 0;
__u64 flags = 0;
int err;
@@ -274,10 +305,10 @@ int bpf_prog7(struct sk_msg_md *msg)
if (bytes)
err2 = bpf_msg_cork_bytes(msg, *bytes);
len1 = (__u64)msg->data_end - (__u64)msg->data;
- start = bpf_map_lookup_elem(&sock_pull_bytes, &zero);
- end = bpf_map_lookup_elem(&sock_pull_bytes, &one);
- if (start && end) {
+ start = bpf_map_lookup_elem(&sock_bytes, &zero);
+ end = bpf_map_lookup_elem(&sock_bytes, &one);
+ if (start && end) {
bpf_printk("sk_msg2: pull(%i:%i)\n",
start ? *start : 0, end ? *end : 0);
err = bpf_msg_pull_data(msg, *start, *end, 0);
@@ -288,6 +319,22 @@ int bpf_prog7(struct sk_msg_md *msg)
bpf_printk("sk_msg2: length update %i->%i\n",
len1, len2);
}
+
+ start_push = bpf_map_lookup_elem(&sock_bytes, &two);
+ end_push = bpf_map_lookup_elem(&sock_bytes, &three);
+ if (start_push && end_push) {
+ bpf_printk("sk_msg4: push(%i:%i)\n",
+ start_push ? *start_push : 0,
+ end_push ? *end_push : 0);
+ err = bpf_msg_push_data(msg, *start_push, *end_push, 0);
+ if (err)
+ bpf_printk("sk_msg4: push_data err %i\n",
+ err);
+ len3 = (__u64)msg->data_end - (__u64)msg->data;
+ bpf_printk("sk_msg4: length push_update %i->%i\n",
+ len2 ? len2 : len1, len3);
+ }
+
f = bpf_map_lookup_elem(&sock_redir_flags, &zero);
if (f && *f) {
key = 2;
@@ -342,8 +389,8 @@ int bpf_prog9(struct sk_msg_md *msg)
SEC("sk_msg7")
int bpf_prog10(struct sk_msg_md *msg)
{
- int *bytes, zero = 0, one = 1;
- int *start, *end;
+ int *bytes, *start, *end, *start_push, *end_push;
+ int zero = 0, one = 1, two = 2, three = 3;
bytes = bpf_map_lookup_elem(&sock_apply_bytes, &zero);
if (bytes)
@@ -351,10 +398,14 @@ int bpf_prog10(struct sk_msg_md *msg)
bytes = bpf_map_lookup_elem(&sock_cork_bytes, &zero);
if (bytes)
bpf_msg_cork_bytes(msg, *bytes);
- start = bpf_map_lookup_elem(&sock_pull_bytes, &zero);
- end = bpf_map_lookup_elem(&sock_pull_bytes, &one);
+ start = bpf_map_lookup_elem(&sock_bytes, &zero);
+ end = bpf_map_lookup_elem(&sock_bytes, &one);
if (start && end)
bpf_msg_pull_data(msg, *start, *end, 0);
+ start_push = bpf_map_lookup_elem(&sock_bytes, &two);
+ end_push = bpf_map_lookup_elem(&sock_bytes, &three);
+ if (start_push && end_push)
+ bpf_msg_push_data(msg, *start_push, *end_push, 0);
return SK_DROP;
}
--
1.9.1
^ permalink raw reply related
* [bpf-next v2 2/3] bpf: libbpf support for msg_push_data
From: John Fastabend @ 2018-10-20 2:56 UTC (permalink / raw)
To: ast, daniel; +Cc: john.fastabend, netdev
In-Reply-To: <1540004211-7647-1-git-send-email-john.fastabend@gmail.com>
Add support for new bpf_msg_push_data in libbpf.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
tools/include/uapi/linux/bpf.h | 20 +++++++++++++++++++-
tools/testing/selftests/bpf/bpf_helpers.h | 2 ++
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index a2fb333..852dc17 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2240,6 +2240,23 @@ struct bpf_stack_build_id {
* pointer that was returned from bpf_sk_lookup_xxx\ ().
* Return
* 0 on success, or a negative error in case of failure.
+ *
+ * int bpf_msg_push_data(struct sk_buff *skb, u32 start, u32 len, u64 flags)
+ * Description
+ * For socket policies, insert *len* bytes into msg at offset
+ * *start*.
+ *
+ * If a program of type **BPF_PROG_TYPE_SK_MSG** is run on a
+ * *msg* it may want to insert metadata or options into the msg.
+ * This can later be read and used by any of the lower layer BPF
+ * hooks.
+ *
+ * This helper may fail if under memory pressure (a malloc
+ * fails) in these cases BPF programs will get an appropriate
+ * error and BPF programs will need to handle them.
+ *
+ * Return
+ * 0 on success, or a negative error in case of failure.
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -2331,7 +2348,8 @@ struct bpf_stack_build_id {
FN(sk_release), \
FN(map_push_elem), \
FN(map_pop_elem), \
- FN(map_peek_elem),
+ FN(map_peek_elem), \
+ FN(msg_push_data),
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index 6407a3d..686e57c 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -111,6 +111,8 @@ static int (*bpf_msg_cork_bytes)(void *ctx, int len) =
(void *) BPF_FUNC_msg_cork_bytes;
static int (*bpf_msg_pull_data)(void *ctx, int start, int end, int flags) =
(void *) BPF_FUNC_msg_pull_data;
+static int (*bpf_msg_push_data)(void *ctx, int start, int end, int flags) =
+ (void *) BPF_FUNC_msg_push_data;
static int (*bpf_bind)(void *ctx, void *addr, int addr_len) =
(void *) BPF_FUNC_bind;
static int (*bpf_xdp_adjust_tail)(void *ctx, int offset) =
--
1.9.1
^ permalink raw reply related
* [bpf-next v2 1/3] bpf: sk_msg program helper bpf_msg_push_data
From: John Fastabend @ 2018-10-20 2:56 UTC (permalink / raw)
To: ast, daniel; +Cc: john.fastabend, netdev
In-Reply-To: <1540004211-7647-1-git-send-email-john.fastabend@gmail.com>
This allows user to push data into a msg using sk_msg program types.
The format is as follows,
bpf_msg_push_data(msg, offset, len, flags)
this will insert 'len' bytes at offset 'offset'. For example to
prepend 10 bytes at the front of the message the user can,
bpf_msg_push_data(msg, 0, 10, 0);
This will invalidate data bounds so BPF user will have to then recheck
data bounds after calling this. After this the msg size will have been
updated and the user is free to write into the added bytes. We allow
any offset/len as long as it is within the (data, data_end) range.
However, a copy will be required if the ring is full and its possible
for the helper to fail with ENOMEM or EINVAL errors which need to be
handled by the BPF program.
This can be used similar to XDP metadata to pass data between sk_msg
layer and lower layers.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/linux/skmsg.h | 5 ++
include/uapi/linux/bpf.h | 20 ++++++-
net/core/filter.c | 134 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 158 insertions(+), 1 deletion(-)
diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h
index 84e1886..2a11e9d 100644
--- a/include/linux/skmsg.h
+++ b/include/linux/skmsg.h
@@ -207,6 +207,11 @@ static inline struct scatterlist *sk_msg_elem(struct sk_msg *msg, int which)
return &msg->sg.data[which];
}
+static inline struct scatterlist sk_msg_elem_cpy(struct sk_msg *msg, int which)
+{
+ return msg->sg.data[which];
+}
+
static inline struct page *sk_msg_page(struct sk_msg *msg, int which)
{
return sg_page(sk_msg_elem(msg, which));
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index a2fb333..852dc17 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2240,6 +2240,23 @@ struct bpf_stack_build_id {
* pointer that was returned from bpf_sk_lookup_xxx\ ().
* Return
* 0 on success, or a negative error in case of failure.
+ *
+ * int bpf_msg_push_data(struct sk_buff *skb, u32 start, u32 len, u64 flags)
+ * Description
+ * For socket policies, insert *len* bytes into msg at offset
+ * *start*.
+ *
+ * If a program of type **BPF_PROG_TYPE_SK_MSG** is run on a
+ * *msg* it may want to insert metadata or options into the msg.
+ * This can later be read and used by any of the lower layer BPF
+ * hooks.
+ *
+ * This helper may fail if under memory pressure (a malloc
+ * fails) in these cases BPF programs will get an appropriate
+ * error and BPF programs will need to handle them.
+ *
+ * Return
+ * 0 on success, or a negative error in case of failure.
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -2331,7 +2348,8 @@ struct bpf_stack_build_id {
FN(sk_release), \
FN(map_push_elem), \
FN(map_pop_elem), \
- FN(map_peek_elem),
+ FN(map_peek_elem), \
+ FN(msg_push_data),
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
diff --git a/net/core/filter.c b/net/core/filter.c
index 5fd5139..35c6933 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2297,6 +2297,137 @@ int skb_do_redirect(struct sk_buff *skb)
.arg4_type = ARG_ANYTHING,
};
+BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start,
+ u32, len, u64, flags)
+{
+ struct scatterlist sge, nsge, nnsge, rsge = {0}, *psge;
+ u32 new, i = 0, l, space, copy = 0, offset = 0;
+ u8 *raw, *to, *from;
+ struct page *page;
+
+ if (unlikely(flags))
+ return -EINVAL;
+
+ /* First find the starting scatterlist element */
+ i = msg->sg.start;
+ do {
+ l = sk_msg_elem(msg, i)->length;
+
+ if (start < offset + l)
+ break;
+ offset += l;
+ sk_msg_iter_var_next(i);
+ } while (i != msg->sg.end);
+
+ if (start >= offset + l)
+ return -EINVAL;
+
+ space = MAX_MSG_FRAGS - sk_msg_elem_used(msg);
+
+ /* If no space available will fallback to copy, we need at
+ * least one scatterlist elem available to push data into
+ * when start aligns to the beginning of an element or two
+ * when it falls inside an element. We handle the start equals
+ * offset case because its the common case for inserting a
+ * header.
+ */
+ if (!space || (space == 1 && start != offset))
+ copy = msg->sg.data[i].length;
+
+ page = alloc_pages(__GFP_NOWARN | GFP_ATOMIC | __GFP_COMP,
+ get_order(copy + len));
+ if (unlikely(!page))
+ return -ENOMEM;
+
+ if (copy) {
+ int front, back;
+
+ raw = page_address(page);
+
+ psge = sk_msg_elem(msg, i);
+ front = start - offset;
+ back = psge->length - front;
+ from = sg_virt(psge);
+
+ if (front)
+ memcpy(raw, from, front);
+
+ if (back) {
+ from += front;
+ to = raw + front + len;
+
+ memcpy(to, from, back);
+ }
+
+ put_page(sg_page(psge));
+ } else if (start - offset) {
+ psge = sk_msg_elem(msg, i);
+ rsge = sk_msg_elem_cpy(msg, i);
+
+ psge->length = start - offset;
+ rsge.length -= psge->length;
+ rsge.offset += start;
+
+ sk_msg_iter_var_next(i);
+ sg_unmark_end(psge);
+ sk_msg_iter_next(msg, end);
+ }
+
+ /* Slot(s) to place newly allocated data */
+ new = i;
+
+ /* Shift one or two slots as needed */
+ if (!copy) {
+ sge = sk_msg_elem_cpy(msg, i);
+
+ sk_msg_iter_var_next(i);
+ sg_unmark_end(&sge);
+ sk_msg_iter_next(msg, end);
+
+ nsge = sk_msg_elem_cpy(msg, i);
+ if (rsge.length) {
+ sk_msg_iter_var_next(i);
+ nnsge = sk_msg_elem_cpy(msg, i);
+ }
+
+ while (i != msg->sg.end) {
+ msg->sg.data[i] = sge;
+ sge = nsge;
+ sk_msg_iter_var_next(i);
+ if (rsge.length) {
+ nsge = nnsge;
+ nnsge = sk_msg_elem_cpy(msg, i);
+ } else {
+ nsge = sk_msg_elem_cpy(msg, i);
+ }
+ }
+ }
+
+ /* Place newly allocated data buffer */
+ sk_mem_charge(msg->sk, len);
+ msg->sg.size += len;
+ msg->sg.copy[new] = false;
+ sg_set_page(&msg->sg.data[new], page, len + copy, 0);
+ if (rsge.length) {
+ get_page(sg_page(&rsge));
+ sk_msg_iter_var_next(new);
+ msg->sg.data[new] = rsge;
+ }
+
+ sk_msg_compute_data_pointers(msg);
+ return 0;
+}
+
+static const struct bpf_func_proto bpf_msg_push_data_proto = {
+ .func = bpf_msg_push_data,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+ .arg2_type = ARG_ANYTHING,
+ .arg3_type = ARG_ANYTHING,
+ .arg4_type = ARG_ANYTHING,
+};
+
BPF_CALL_1(bpf_get_cgroup_classid, const struct sk_buff *, skb)
{
return task_get_classid(skb);
@@ -4854,6 +4985,7 @@ bool bpf_helper_changes_pkt_data(void *func)
func == bpf_xdp_adjust_head ||
func == bpf_xdp_adjust_meta ||
func == bpf_msg_pull_data ||
+ func == bpf_msg_push_data ||
func == bpf_xdp_adjust_tail ||
#if IS_ENABLED(CONFIG_IPV6_SEG6_BPF)
func == bpf_lwt_seg6_store_bytes ||
@@ -5130,6 +5262,8 @@ bool bpf_helper_changes_pkt_data(void *func)
return &bpf_msg_cork_bytes_proto;
case BPF_FUNC_msg_pull_data:
return &bpf_msg_pull_data_proto;
+ case BPF_FUNC_msg_push_data:
+ return &bpf_msg_push_data_proto;
case BPF_FUNC_get_local_storage:
return &bpf_get_local_storage_proto;
default:
--
1.9.1
^ permalink raw reply related
* [bpf-next v2 0/3] sockmap, bpf_msg_push_data helper
From: John Fastabend @ 2018-10-20 2:56 UTC (permalink / raw)
To: ast, daniel; +Cc: john.fastabend, netdev
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(-)
--
1.9.1
^ permalink raw reply
* Re: [PATCH net-next] net: loopback: clear skb->tstamp before netif_rx()
From: Soheil Hassas Yeganeh @ 2018-10-20 2:29 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Eric Dumazet, Willem de Bruijn
In-Reply-To: <20181020021126.61472-1-edumazet@google.com>
On Fri, Oct 19, 2018 at 10:11 PM Eric Dumazet <edumazet@google.com> wrote:
>
> At least UDP / TCP stacks can now cook skbs with a tstamp using
> MONOTONIC base (or arbitrary values with SCM_TXTIME)
>
> Since loopback driver does not call (directly or indirectly)
> skb_scrub_packet(), we need to clear skb->tstamp so that
> net_timestamp_check() can eventually resample the time,
> using ktime_get_real().
>
> Fixes: 80b14dee2bea ("net: Add a new socket option for a future transmit time.")
> Fixes: fb420d5d91c1 ("tcp/fq: move back to CLOCK_MONOTONIC")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Willem de Bruijn <willemb@google.com>
> Cc: Soheil Hassas Yeganeh <soheil@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
Thank you, Eric!
> ---
> drivers/net/loopback.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
> index a7207fa7e451311aed13cdeb100e0ea7922931bf..2df7f60fe05220c19896a251b6b15239f4b95112 100644
> --- a/drivers/net/loopback.c
> +++ b/drivers/net/loopback.c
> @@ -69,6 +69,10 @@ static netdev_tx_t loopback_xmit(struct sk_buff *skb,
> int len;
>
> skb_tx_timestamp(skb);
> +
> + /* do not fool net_timestamp_check() with various clock bases */
> + skb->tstamp = 0;
> +
> skb_orphan(skb);
>
> /* Before queueing this packet to netif_rx(),
> --
> 2.19.1.568.g152ad8e336-goog
>
^ permalink raw reply
* [PATCH net-next] net: loopback: clear skb->tstamp before netif_rx()
From: Eric Dumazet @ 2018-10-20 2:11 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Eric Dumazet, Eric Dumazet, Willem de Bruijn,
Soheil Hassas Yeganeh
At least UDP / TCP stacks can now cook skbs with a tstamp using
MONOTONIC base (or arbitrary values with SCM_TXTIME)
Since loopback driver does not call (directly or indirectly)
skb_scrub_packet(), we need to clear skb->tstamp so that
net_timestamp_check() can eventually resample the time,
using ktime_get_real().
Fixes: 80b14dee2bea ("net: Add a new socket option for a future transmit time.")
Fixes: fb420d5d91c1 ("tcp/fq: move back to CLOCK_MONOTONIC")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Willem de Bruijn <willemb@google.com>
Cc: Soheil Hassas Yeganeh <soheil@google.com>
---
drivers/net/loopback.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index a7207fa7e451311aed13cdeb100e0ea7922931bf..2df7f60fe05220c19896a251b6b15239f4b95112 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -69,6 +69,10 @@ static netdev_tx_t loopback_xmit(struct sk_buff *skb,
int len;
skb_tx_timestamp(skb);
+
+ /* do not fool net_timestamp_check() with various clock bases */
+ skb->tstamp = 0;
+
skb_orphan(skb);
/* Before queueing this packet to netif_rx(),
--
2.19.1.568.g152ad8e336-goog
^ permalink raw reply related
* RE: [Intel-wired-lan] [PATCH] igb: shorten maximum PHC timecounter update interval
From: Brown, Aaron F @ 2018-10-20 1:13 UTC (permalink / raw)
To: Miroslav Lichvar, intel-wired-lan@lists.osuosl.org,
netdev@vger.kernel.org
Cc: Thomas Gleixner, Richard Cochran
In-Reply-To: <20181012111339.1361-1-mlichvar@redhat.com>
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On
> Behalf Of Miroslav Lichvar
> Sent: Friday, October 12, 2018 4:14 AM
> To: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org
> Cc: Thomas Gleixner <tglx@linutronix.de>; Richard Cochran
> <richardcochran@gmail.com>
> Subject: [Intel-wired-lan] [PATCH] igb: shorten maximum PHC timecounter
> update interval
>
> The timecounter needs to be updated at least once per ~550 seconds in
> order to avoid a 40-bit SYSTIM timestamp to be misinterpreted as an old
> timestamp.
>
> Since commit 500462a9d ("timers: Switch to a non-cascading wheel"),
> scheduling of delayed work seems to be less accurate and a requested
> delay of 540 seconds may actually be longer than 550 seconds. Shorten
> the delay to 480 seconds to be sure the timecounter is updated in time.
>
> This fixes an issue with HW timestamps on 82580/I350/I354 being off by
> ~1100 seconds for few seconds every ~9 minutes.
>
> Cc: Jacob Keller <jacob.e.keller@intel.com>
> Cc: Richard Cochran <richardcochran@gmail.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Miroslav Lichvar <mlichvar@redhat.com>
> ---
> drivers/net/ethernet/intel/igb/igb_ptp.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
^ permalink raw reply
* Re: [PATCH net] net: fix pskb_trim_rcsum_slow() with odd trim offset
From: Eric Dumazet @ 2018-10-20 0:13 UTC (permalink / raw)
To: Dimitris Michailidis, edumazet, davem; +Cc: netdev
In-Reply-To: <20181020000713.228659-1-dmichail@google.com>
On 10/19/2018 05:07 PM, Dimitris Michailidis wrote:
> We've been getting checksum errors involving small UDP packets, usually
> 59B packets with 1 extra non-zero padding byte. netdev_rx_csum_fault()
> has been complaining that HW is providing bad checksums. Turns out the
> problem is in pskb_trim_rcsum_slow(), introduced in commit 88078d98d1bb
> ("net: pskb_trim_rcsum() and CHECKSUM_COMPLETE are friends").
>
> The source of the problem is that when the bytes we are trimming start
> at an odd address, as in the case of the 1 padding byte above,
> skb_checksum() returns a byte-swapped value. We cannot just combine this
> with skb->csum using csum_sub(). We need to use csum_block_sub() here
> that takes into account the parity of the start address and handles the
> swapping.
>
> Matches existing code in __skb_postpull_rcsum() and esp_remove_trailer().
>
> Fixes: 88078d98d1bb ("net: pskb_trim_rcsum() and CHECKSUM_COMPLETE are friends")
> Signed-off-by: Dimitris Michailidis <dmichail@google.com>
Thanks a lot Dimitris for finding this.
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* [PATCH net] net: fix pskb_trim_rcsum_slow() with odd trim offset
From: Dimitris Michailidis @ 2018-10-20 0:07 UTC (permalink / raw)
To: edumazet, davem; +Cc: netdev, Dimitris Michailidis
We've been getting checksum errors involving small UDP packets, usually
59B packets with 1 extra non-zero padding byte. netdev_rx_csum_fault()
has been complaining that HW is providing bad checksums. Turns out the
problem is in pskb_trim_rcsum_slow(), introduced in commit 88078d98d1bb
("net: pskb_trim_rcsum() and CHECKSUM_COMPLETE are friends").
The source of the problem is that when the bytes we are trimming start
at an odd address, as in the case of the 1 padding byte above,
skb_checksum() returns a byte-swapped value. We cannot just combine this
with skb->csum using csum_sub(). We need to use csum_block_sub() here
that takes into account the parity of the start address and handles the
swapping.
Matches existing code in __skb_postpull_rcsum() and esp_remove_trailer().
Fixes: 88078d98d1bb ("net: pskb_trim_rcsum() and CHECKSUM_COMPLETE are friends")
Signed-off-by: Dimitris Michailidis <dmichail@google.com>
---
net/core/skbuff.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 428094b577fc..f817f336595d 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1846,8 +1846,9 @@ int pskb_trim_rcsum_slow(struct sk_buff *skb, unsigned int len)
if (skb->ip_summed == CHECKSUM_COMPLETE) {
int delta = skb->len - len;
- skb->csum = csum_sub(skb->csum,
- skb_checksum(skb, len, delta, 0));
+ skb->csum = csum_block_sub(skb->csum,
+ skb_checksum(skb, len, delta, 0),
+ len);
}
return __pskb_trim(skb, len);
}
--
2.19.1.568.g152ad8e336-goog
^ permalink raw reply related
* Re: [PATCH] net: ethernet: lpc_eth: add device and device node local variables
From: David Miller @ 2018-10-20 0:05 UTC (permalink / raw)
To: vz; +Cc: slemieux.tyco, netdev
In-Reply-To: <20181018232511.17325-1-vz@mleia.com>
From: Vladimir Zapolskiy <vz@mleia.com>
Date: Fri, 19 Oct 2018 02:25:11 +0300
> Trivial non-functional change added to simplify getting multiple
> references to device pointer in lpc_eth_drv_probe().
>
> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: ethernet: lpc_eth: remove unused local variable
From: David Miller @ 2018-10-20 0:05 UTC (permalink / raw)
To: vz; +Cc: slemieux.tyco, netdev
In-Reply-To: <20181018230653.10637-1-vz@mleia.com>
From: Vladimir Zapolskiy <vz@mleia.com>
Date: Fri, 19 Oct 2018 02:06:53 +0300
> A trivial change which removes an unused local variable, the issue
> is reported as a compile time warning:
>
> drivers/net/ethernet/nxp/lpc_eth.c: In function 'lpc_eth_drv_probe':
> drivers/net/ethernet/nxp/lpc_eth.c:1250:21: warning: variable 'phydev' set but not used [-Wunused-but-set-variable]
> struct phy_device *phydev;
> ^~~~~~
>
> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: ethernet: lpc_eth: remove CONFIG_OF guard from the driver
From: David Miller @ 2018-10-20 0:05 UTC (permalink / raw)
To: vz; +Cc: slemieux.tyco, netdev
In-Reply-To: <20181018225841.17835-1-vz@mleia.com>
From: Vladimir Zapolskiy <vz@mleia.com>
Date: Fri, 19 Oct 2018 01:58:41 +0300
> The MAC controller device is available on NXP LPC32xx platform only,
> and the LPC32xx platform supports OF builds only, so additional
> checks in the device driver are not needed.
>
> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: ethernet: lpc_eth: clean up the list of included headers
From: David Miller @ 2018-10-20 0:05 UTC (permalink / raw)
To: vz; +Cc: slemieux.tyco, netdev
In-Reply-To: <20181018225325.4959-1-vz@mleia.com>
From: Vladimir Zapolskiy <vz@mleia.com>
Date: Fri, 19 Oct 2018 01:53:25 +0300
> The change removes all unnecessary included headers from the driver
> source code, the remaining list is sorted in alphabetical order.
>
> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next v2] netpoll: allow cleanup to be synchronous
From: David Miller @ 2018-10-19 23:58 UTC (permalink / raw)
To: dbanerje; +Cc: nhorman, netdev
In-Reply-To: <20181018151826.8373-1-dbanerje@akamai.com>
From: Debabrata Banerjee <dbanerje@akamai.com>
Date: Thu, 18 Oct 2018 11:18:26 -0400
> This fixes a problem introduced by:
> commit 2cde6acd49da ("netpoll: Fix __netpoll_rcu_free so that it can hold the rtnl lock")
>
> When using netconsole on a bond, __netpoll_cleanup can asynchronously
> recurse multiple times, each __netpoll_free_async call can result in
> more __netpoll_free_async's. This means there is now a race between
> cleanup_work queues on multiple netpoll_info's on multiple devices and
> the configuration of a new netpoll. For example if a netconsole is set
> to enable 0, reconfigured, and enable 1 immediately, this netconsole
> will likely not work.
>
> Given the reason for __netpoll_free_async is it can be called when rtnl
> is not locked, if it is locked, we should be able to execute
> synchronously. It appears to be locked everywhere it's called from.
>
> Generalize the design pattern from the teaming driver for current
> callers of __netpoll_free_async.
>
> CC: Neil Horman <nhorman@tuxdriver.com>
> CC: "David S. Miller" <davem@davemloft.net>
> Signed-off-by: Debabrata Banerjee <dbanerje@akamai.com>
Applied, thank you.
^ permalink raw reply
* Re: [bpf-next v3 0/2] Fix kcm + sockmap by checking psock type
From: John Fastabend @ 2018-10-19 23:38 UTC (permalink / raw)
To: Daniel Borkmann, ast, eric.dumazet; +Cc: netdev
In-Reply-To: <81286597-9886-84e0-dcb4-00c15c85fed0@iogearbox.net>
On 10/19/2018 03:57 PM, Daniel Borkmann wrote:
> On 10/20/2018 12:51 AM, Daniel Borkmann wrote:
>> On 10/18/2018 10:58 PM, John Fastabend wrote:
>>> We check if the sk_user_data (the psock in skmsg) is in fact a sockmap
>>> type to late, after we read the refcnt which is an error. This
>>> series moves the check up before reading refcnt and also adds a test
>>> to test_maps to test trying to add a KCM socket into a sockmap.
>>>
>>> While reviewig this code I also found an issue with KCM and kTLS
>>> where each uses sk_data_ready hooks and associated stream parser
>>> breaking expectations in kcm, ktls or both. But that fix will need
>>> to go to net.
>>>
>>> Thanks to Eric for reporting.
>>>
>>> v2: Fix up file +/- my scripts lost track of them
>>> v3: return EBUSY if refcnt is zero
>>>
>>> John Fastabend (2):
>>> bpf: skmsg, fix psock create on existing kcm/tls port
>>> bpf: test_maps add a test to catch kcm + sockmap
>>>
>>> include/linux/skmsg.h | 25 +++++++++---
>>> net/core/sock_map.c | 11 +++---
>>> tools/testing/selftests/bpf/Makefile | 2 +-
>>> tools/testing/selftests/bpf/sockmap_kcm.c | 14 +++++++
>>> tools/testing/selftests/bpf/test_maps.c | 64 ++++++++++++++++++++++++++++++-
>>> 5 files changed, 103 insertions(+), 13 deletions(-)
>>> create mode 100644 tools/testing/selftests/bpf/sockmap_kcm.c
>>
>> Applied, thanks!
>
> Fyi, I've only applied patch 1/2 for now to get the bug fixed. The patch 2/2 throws
> a bunch of warnings that look like the below. Also, I think we leak kcm socket in
> error paths and once we're done with testing, so would be good to close it once
> unneeded. Please respin the test as a stand-alone commit, thanks:
>
Thanks, I didn't see the warnings below locally but will look
into spinning a good version tonight with the closing sock fix
as well.
John
> [...]
> bpf-next/tools/testing/selftests/bpf/libbpf.a -lcap -lelf -lrt -lpthread -o /home/darkstar/trees/bpf-next-ok/tools/testing/selftests/bpf/test_maps
> test_maps.c: In function ‘test_sockmap’:
> test_maps.c:869:0: warning: "AF_KCM" redefined
> #define AF_KCM 41
>
> In file included from /usr/include/sys/socket.h:38:0,
> from test_maps.c:21:
> /usr/include/bits/socket.h:133:0: note: this is the location of the previous definition
> #define AF_KCM PF_KCM
>
^ permalink raw reply
* Re: [PATCH bpf-next v2 02/13] bpf: btf: Add BTF_KIND_FUNC and BTF_KIND_FUNC_PROTO
From: Martin Lau @ 2018-10-19 23:27 UTC (permalink / raw)
To: Edward Cree
Cc: Yonghong Song, Alexei Starovoitov, daniel@iogearbox.net,
netdev@vger.kernel.org, Kernel Team
In-Reply-To: <7d251102-2073-98b7-94a6-4dfcc21a3071@solarflare.com>
On Fri, Oct 19, 2018 at 10:26:53PM +0100, Edward Cree wrote:
> On 19/10/18 20:36, Martin Lau wrote:
> > On Fri, Oct 19, 2018 at 06:04:11PM +0100, Edward Cree wrote:
> >> But you *do* have such a new section.
> >> The patch comment talks about a 'FuncInfo Table' which appears to
> > Note that the new section, which contains the FuncInfo Table,
> > is in a new ELF section ".BTF.ext" instead of the ".BTF".
> > It is not in the ".BTF" section because it is only useful during
> > bpf_prog_load().
> I thought it was because it needed to be munged by the loader/linker?
>
> > IIUC, I think what you are suggesting here is to use (type_id, name)
> > to describe DW_TAG_subprogram "int foo1(int) {}", "int foo2(int) {}",
> > "int foo3(int) {}" where type_id here is referring to the same
> > DW_TAG_subroutine_type, and only define that _one_
> > DW_TAG_subroutine_type in the BTF "type" section.
> Yes, something like that.
>
> > If the concern is having both FUNC and FUNC_PROTO is confusing,
> The concern is that you're conflating different entities (types
> and instances); FUNC_PROTO is just a symptom/canary of that.
>
> > we could go back to the CTF way which adds a new function section
> > in ".BTF" and it is only for DW_TAG_subprogram.
> > BTF_KIND_FUNC_PROTO is then no longer necessary.
> > Some of new BTF verifier checkings may actually go away also.
> > The down side is there will be two id spaces.
> Two id spaces... one for types and the other for subprograms.
> These are different things, so why would you _want_ them to share
> an id space? I don't, for instance, see any situation in which
> you'd want some other record to have a field that could reference
> either.
> And the 'subprogram id' doesn't have to be just for subprograms;
> it could be for instances generally — like I've been saying, a
> variable declaration is to an object type what a subprogram is to
> a function type, just with a few complications like "subprograms
> can only appear at file scope, not nested in other functions" and
> "variables of function type are immutable".
> (I'm assuming that at some point we're going to want to be able to
> have BTF information for e.g. variables stored on a subprogram's
> stack, if only for stuff like single-stepping in a debugger in
> userspace with some sort of mock. At that point, the variable
> has to have its own record — you can't just have some sort of
> magic type record because e.g. "struct foo bar;" has two names,
> one for the type and one for the variable.)
>
btf_type is not exactly a C type.
btf_type is a debug-info. Each btf_type carries specific
debug information. Name is part of the debug-info/btf_type.
If something carries different debug-info, it is another btf_type.
Like struct, the member's names of struct is part of the btf_type.
A struct with the same member's types but different member's names
is a different btf_type.
The same go for function. The function with different function
names and arg names is a different btf_type.
> > Discussed a bit offline with folks about the two id spaces
> > situation and it is not good for debugging purpose.
> Could you unpack this a bit more?
Having two id spaces for debug-info is confusing. They are
all debug-info at the end.
^ permalink raw reply
* Re: [PATCH ghak90 (was ghak32) V4 08/10] audit: add support for containerid to network namespaces
From: Paul Moore @ 2018-10-19 23:18 UTC (permalink / raw)
To: rgb
Cc: containers, linux-api, linux-audit, linux-fsdevel, linux-kernel,
netdev, netfilter-devel, ebiederm, luto, carlos, dhowells, viro,
simo, Eric Paris, Serge Hallyn
In-Reply-To: <5a2b4aadf6994f622bc1ad27a8a6889c7e61edff.1533065887.git.rgb@redhat.com>
On Tue, Jul 31, 2018 at 4:12 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> Audit events could happen in a network namespace outside of a task
> context due to packets received from the net that trigger an auditing
> rule prior to being associated with a running task. The network
> namespace could in use by multiple containers by association to the
> tasks in that network namespace. We still want a way to attribute
> these events to any potential containers. Keep a list per network
> namespace to track these audit container identifiiers.
>
> Add/increment the audit container identifier on:
> - initial setting of the audit container identifier via /proc
> - clone/fork call that inherits an audit container identifier
> - unshare call that inherits an audit container identifier
> - setns call that inherits an audit container identifier
> Delete/decrement the audit container identifier on:
> - an inherited audit container identifier dropped when child set
> - process exit
> - unshare call that drops a net namespace
> - setns call that drops a net namespace
>
> See: https://github.com/linux-audit/audit-kernel/issues/92
> See: https://github.com/linux-audit/audit-testsuite/issues/64
> See: https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Container-ID
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> include/linux/audit.h | 17 ++++++++++
> kernel/audit.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++
> kernel/auditsc.c | 8 ++++-
> kernel/nsproxy.c | 4 +++
> 4 files changed, 114 insertions(+), 1 deletion(-)
...
> @@ -308,6 +312,86 @@ static struct sock *audit_get_sk(const struct net *net)
> return aunet->sk;
> }
>
> +/**
> + * audit_get_netns_contid_list - Return the audit container ID list for the given network namespace
> + * @net: the destination network namespace
> + *
> + * Description:
> + * Returns the list pointer if valid, NULL otherwise. The caller must ensure
> + * that a reference is held for the network namespace while the sock is in use.
> + */
> +struct list_head *audit_get_netns_contid_list(const struct net *net)
> +{
> + struct audit_net *aunet = net_generic(net, audit_net_id);
> +
> + return &aunet->contid_list;
> +}
> +
> +spinlock_t *audit_get_netns_contid_list_lock(const struct net *net)
> +{
> + struct audit_net *aunet = net_generic(net, audit_net_id);
> +
> + return &aunet->contid_list_lock;
> +}
Instead of returning the spinlock, just do away with the
audit_get_ns_contid_list_lock() function and create two separate lock
and unlock functions that basically do the net_generic() and spinlock
operations together, for example:
static int audit_netns_contid_lock(const struct net *net)
{
aunet = net_generic(net, audit_net_id);
if (!aunet)
return -whatever;
spin_lock(aunet->lock);
return 0;
}
> +void audit_netns_contid_add(struct net *net, u64 contid)
> +{
> + spinlock_t *lock = audit_get_netns_contid_list_lock(net);
> + struct list_head *contid_list = audit_get_netns_contid_list(net);
> + struct audit_contid *cont;
> +
> + if (!audit_contid_valid(contid))
> + return;
> + spin_lock(lock);
> + if (!list_empty(contid_list))
> + list_for_each_entry(cont, contid_list, list)
> + if (cont->id == contid) {
> + refcount_inc(&cont->refcount);
> + goto out;
> + }
> + cont = kmalloc(sizeof(struct audit_contid), GFP_KERNEL);
> + if (cont) {
> + INIT_LIST_HEAD(&cont->list);
> + cont->id = contid;
> + refcount_set(&cont->refcount, 1);
> + list_add(&cont->list, contid_list);
> + }
> +out:
> + spin_unlock(lock);
> +}
> +
> +void audit_netns_contid_del(struct net *net, u64 contid)
> +{
> + spinlock_t *lock = audit_get_netns_contid_list_lock(net);
> + struct list_head *contid_list = audit_get_netns_contid_list(net);
> + struct audit_contid *cont = NULL;
> +
> + if (!audit_contid_valid(contid))
> + return;
> + spin_lock(lock);
> + if (!list_empty(contid_list))
> + list_for_each_entry(cont, contid_list, list)
> + if (cont->id == contid) {
> + list_del(&cont->list);
> + if (refcount_dec_and_test(&cont->refcount))
> + kfree(cont);
> + break;
> + }
> + spin_unlock(lock);
> +}
> +
> +void audit_switch_task_namespaces(struct nsproxy *ns, struct task_struct *p)
> +{
> + u64 contid = audit_get_contid(p);
> + struct nsproxy *new = p->nsproxy;
> +
> + if (!audit_contid_valid(contid))
> + return;
> + audit_netns_contid_del(ns->net_ns, contid);
> + if (new)
> + audit_netns_contid_add(new->net_ns, contid);
> +}
> +
> void audit_panic(const char *message)
> {
> switch (audit_failure) {
> @@ -1547,6 +1631,8 @@ static int __net_init audit_net_init(struct net *net)
> return -ENOMEM;
> }
> aunet->sk->sk_sndtimeo = MAX_SCHEDULE_TIMEOUT;
> + INIT_LIST_HEAD(&aunet->contid_list);
> + spin_lock_init(&aunet->contid_list_lock);
>
> return 0;
> }
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 610c6869..fdf3f68 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -75,6 +75,7 @@
> #include <linux/uaccess.h>
> #include <linux/fsnotify_backend.h>
> #include <uapi/linux/limits.h>
> +#include <net/net_namespace.h>
>
> #include "audit.h"
>
> @@ -2165,6 +2166,7 @@ int audit_set_contid(struct task_struct *task, u64 contid)
> uid_t uid;
> struct tty_struct *tty;
> char comm[sizeof(current->comm)];
> + struct net *net = task->nsproxy->net_ns;
>
> task_lock(task);
> /* Can't set if audit disabled */
> @@ -2186,8 +2188,12 @@ int audit_set_contid(struct task_struct *task, u64 contid)
> else if (!(thread_group_leader(task) && thread_group_empty(task)))
> rc = -EALREADY;
> read_unlock(&tasklist_lock);
> - if (!rc)
> + if (!rc) {
> + if (audit_contid_valid(oldcontid))
> + audit_netns_contid_del(net, oldcontid);
> task->audit->contid = contid;
> + audit_netns_contid_add(net, contid);
> + }
> task_unlock(task);
>
> if (!audit_enabled)
> diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c
> index f6c5d33..718b120 100644
> --- a/kernel/nsproxy.c
> +++ b/kernel/nsproxy.c
> @@ -27,6 +27,7 @@
> #include <linux/syscalls.h>
> #include <linux/cgroup.h>
> #include <linux/perf_event.h>
> +#include <linux/audit.h>
>
> static struct kmem_cache *nsproxy_cachep;
>
> @@ -140,6 +141,7 @@ int copy_namespaces(unsigned long flags, struct task_struct *tsk)
> struct nsproxy *old_ns = tsk->nsproxy;
> struct user_namespace *user_ns = task_cred_xxx(tsk, user_ns);
> struct nsproxy *new_ns;
> + u64 contid = audit_get_contid(tsk);
>
> if (likely(!(flags & (CLONE_NEWNS | CLONE_NEWUTS | CLONE_NEWIPC |
> CLONE_NEWPID | CLONE_NEWNET |
> @@ -167,6 +169,7 @@ int copy_namespaces(unsigned long flags, struct task_struct *tsk)
> return PTR_ERR(new_ns);
>
> tsk->nsproxy = new_ns;
> + audit_netns_contid_add(new_ns->net_ns, contid);
> return 0;
> }
>
> @@ -224,6 +227,7 @@ void switch_task_namespaces(struct task_struct *p, struct nsproxy *new)
> ns = p->nsproxy;
> p->nsproxy = new;
> task_unlock(p);
> + audit_switch_task_namespaces(ns, p);
>
> if (ns && atomic_dec_and_test(&ns->count))
> free_nsproxy(ns);
^ permalink raw reply
* Re: [PATCH ghak90 (was ghak32) V4 01/10] audit: collect audit task parameters
From: Paul Moore @ 2018-10-19 23:15 UTC (permalink / raw)
To: rgb
Cc: containers, linux-api, linux-audit, linux-fsdevel, linux-kernel,
netdev, netfilter-devel, ebiederm, luto, carlos, dhowells, viro,
simo, Eric Paris, Serge Hallyn
In-Reply-To: <8e617ab568df28a66dfbe3284452de186b42fb0f.1533065887.git.rgb@redhat.com>
On Sun, Aug 5, 2018 at 4:32 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> The audit-related parameters in struct task_struct should ideally be
> collected together and accessed through a standard audit API.
>
> Collect the existing loginuid, sessionid and audit_context together in a
> new struct audit_task_info called "audit" in struct task_struct.
>
> Use kmem_cache to manage this pool of memory.
> Un-inline audit_free() to be able to always recover that memory.
>
> See: https://github.com/linux-audit/audit-kernel/issues/81
>
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> include/linux/audit.h | 34 ++++++++++++++++++++++++----------
> include/linux/sched.h | 5 +----
> init/init_task.c | 3 +--
> init/main.c | 2 ++
> kernel/auditsc.c | 51 ++++++++++++++++++++++++++++++++++++++++++---------
> kernel/fork.c | 4 +++-
> 6 files changed, 73 insertions(+), 26 deletions(-)
...
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 9334fbe..8964332 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -219,8 +219,15 @@ static inline void audit_log_task_info(struct audit_buffer *ab,
>
> /* These are defined in auditsc.c */
> /* Public API */
> +struct audit_task_info {
> + kuid_t loginuid;
> + unsigned int sessionid;
> + struct audit_context *ctx;
> +};
...
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 87bf02d..e117272 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -873,10 +872,8 @@ struct task_struct {
>
> struct callback_head *task_works;
>
> - struct audit_context *audit_context;
> #ifdef CONFIG_AUDITSYSCALL
> - kuid_t loginuid;
> - unsigned int sessionid;
> + struct audit_task_info *audit;
> #endif
> struct seccomp seccomp;
Prior to this patch audit_context was available regardless of
CONFIG_AUDITSYSCALL, after this patch the corresponding audit_context
is only available when CONFIG_AUDITSYSCALL is defined.
> diff --git a/init/main.c b/init/main.c
> index 3b4ada1..6aba171 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -92,6 +92,7 @@
> #include <linux/rodata_test.h>
> #include <linux/jump_label.h>
> #include <linux/mem_encrypt.h>
> +#include <linux/audit.h>
>
> #include <asm/io.h>
> #include <asm/bugs.h>
> @@ -721,6 +722,7 @@ asmlinkage __visible void __init start_kernel(void)
> nsfs_init();
> cpuset_init();
> cgroup_init();
> + audit_task_init();
> taskstats_init_early();
> delayacct_init();
It seems like we would need either init_struct_audit or
audit_task_init(), but not both, yes?
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index fb20746..88779a7 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -841,7 +841,7 @@ static inline struct audit_context *audit_take_context(struct task_struct *tsk,
> int return_valid,
> long return_code)
> {
> - struct audit_context *context = tsk->audit_context;
> + struct audit_context *context = tsk->audit->ctx;
>
> if (!context)
> return NULL;
> @@ -926,6 +926,15 @@ static inline struct audit_context *audit_alloc_context(enum audit_state state)
> return context;
> }
>
> +static struct kmem_cache *audit_task_cache;
> +
> +void __init audit_task_init(void)
> +{
> + audit_task_cache = kmem_cache_create("audit_task",
> + sizeof(struct audit_task_info),
> + 0, SLAB_PANIC, NULL);
> +}
This is somewhat related to the CONFIG_AUDITSYSCALL comment above, but
since the audit_task_info contains generic audit state (not just
syscall related state), it seems like this, and the audit_task_info
accessors/helpers, should live in kernel/audit.c.
There are probably a few other things that should move to
kernel/audit.c too, e.g. audit_alloc(). Have you verified that this
builds/runs correctly on architectures that define CONFIG_AUDIT but
not CONFIG_AUDITSYSCALL?
> /**
> * audit_alloc - allocate an audit context block for a task
> * @tsk: task
> @@ -940,17 +949,28 @@ int audit_alloc(struct task_struct *tsk)
> struct audit_context *context;
> enum audit_state state;
> char *key = NULL;
> + struct audit_task_info *info;
> +
> + info = kmem_cache_zalloc(audit_task_cache, GFP_KERNEL);
> + if (!info)
> + return -ENOMEM;
> + info->loginuid = audit_get_loginuid(current);
> + info->sessionid = audit_get_sessionid(current);
> + tsk->audit = info;
>
> if (likely(!audit_ever_enabled))
> return 0; /* Return if not auditing. */
I don't view this as necessary for initial acceptance, and
synchronization/locking might render this undesirable, but it would be
curious to see if we could do something clever with refcnts and
copy-on-write to minimize the number of kmem_cache objects in use in
the !audit_ever_enabled (and possibly the AUDIT_DISABLED) case.
> state = audit_filter_task(tsk, &key);
> if (state == AUDIT_DISABLED) {
> + audit_set_context(tsk, NULL);
It's already NULL, isn't it?
> clear_tsk_thread_flag(tsk, TIF_SYSCALL_AUDIT);
> return 0;
> }
>
> if (!(context = audit_alloc_context(state))) {
> + tsk->audit = NULL;
> + kmem_cache_free(audit_task_cache, info);
> kfree(key);
> audit_log_lost("out of memory in audit_alloc");
> return -ENOMEM;
> @@ -962,6 +982,12 @@ int audit_alloc(struct task_struct *tsk)
> return 0;
> }
>
> +struct audit_task_info init_struct_audit = {
> + .loginuid = INVALID_UID,
> + .sessionid = AUDIT_SID_UNSET,
> + .ctx = NULL,
> +};
> +
> static inline void audit_free_context(struct audit_context *context)
> {
> audit_free_names(context);
^ permalink raw reply
* Re: [PATCH v8 bpf-next 0/2] bpf: add cg_skb_is_valid_access
From: Alexei Starovoitov @ 2018-10-19 23:09 UTC (permalink / raw)
To: Song Liu; +Cc: netdev, ast, daniel, kernel-team, edumazet
In-Reply-To: <20181019165758.1410213-1-songliubraving@fb.com>
On Fri, Oct 19, 2018 at 09:57:56AM -0700, Song Liu wrote:
> Changes v7 -> v8:
> 1. Dynamically allocate the dummy sk to avoid race conditions.
>
> Changes v6 -> v7:
> 1. Make dummy sk a global variable (test_run_sk).
>
> Changes v5 -> v6:
> 1. Fixed dummy sk in bpf_prog_test_run_skb() as suggested by Eric Dumazet.
>
> Changes v4 -> v5:
> 1. Replaced bpf_compute_and_save_data_pointers() with
> bpf_compute_and_save_data_end();
> Replaced bpf_restore_data_pointers() with bpf_restore_data_end().
> 2. Fixed indentation in test_verifier.c
>
> Changes v3 -> v4:
> 1. Fixed crash issue reported by Alexei.
>
> Changes v2 -> v3:
> 1. Added helper function bpf_compute_and_save_data_pointers() and
> bpf_restore_data_pointers().
>
> Changes v1 -> v2:
> 1. Updated the list of read-only fields, and read-write fields.
> 2. Added dummy sk to bpf_prog_test_run_skb().
>
> This set enables BPF program of type BPF_PROG_TYPE_CGROUP_SKB to access
> some __skb_buff data directly.
Applied, Thanks
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: Extend the sk_lookup() helper to XDP hookpoint.
From: Nitin Hande @ 2018-10-19 23:04 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Joe Stringer, Martin KaFai Lau, netdev, ast, Jesper Brouer,
john fastabend
In-Reply-To: <6c530eaa-c4dd-bcf9-fce5-1f9d66b8efe3@iogearbox.net>
On Fri, 19 Oct 2018 22:32:28 +0200
Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 10/19/2018 06:47 PM, Joe Stringer wrote:
> > On Thu, 18 Oct 2018 at 22:07, Martin Lau <kafai@fb.com> wrote:
> >> On Thu, Oct 18, 2018 at 04:52:40PM -0700, Joe Stringer wrote:
> >>> On Thu, 18 Oct 2018 at 14:20, Daniel Borkmann
> >>> <daniel@iogearbox.net> wrote:
> >>>> On 10/18/2018 11:06 PM, Joe Stringer wrote:
> >>>>> On Thu, 18 Oct 2018 at 11:54, Nitin Hande
> >>>>> <nitin.hande@gmail.com> wrote:
> >>>> [...]
> >>>>>> Open Issue
> >>>>>> * The underlying code relies on presence of an skb to find out
> >>>>>> the right sk for the case of REUSEPORT socket option. Since
> >>>>>> there is no skb available at XDP hookpoint, the helper
> >>>>>> function will return the first available sk based off the 5
> >>>>>> tuple hash. If the desire is to return a particular sk
> >>>>>> matching reuseport_cb function, please suggest way to tackle
> >>>>>> it, which can be addressed in a future commit.
> >>>>
> >>>>>> Signed-off-by: Nitin Hande <Nitin.Hande@gmail.com>
> >>>>>
> >>>>> Thanks Nitin, LGTM overall.
> >>>>>
> >>>>> The REUSEPORT thing suggests that the usage of this helper from
> >>>>> XDP layer may lead to a different socket being selected vs. the
> >>>>> equivalent call at TC hook, or other places where the selection
> >>>>> may occur. This could be a bit counter-intuitive.
> >>>>>
> >>>>> One thought I had to work around this was to introduce a flag,
> >>>>> something like BPF_F_FIND_REUSEPORT_SK_BY_HASH. This flag would
> >>>>> effectively communicate in the API that the bpf_sk_lookup_xxx()
> >>>>> functions will only select a REUSEPORT socket based on the hash
> >>>>> and not by, for example BPF_PROG_TYPE_SK_REUSEPORT programs.
> >>>>> The absence of the flag would support finding REUSEPORT sockets
> >>>>> by other mechanisms (which would be allowed for now from TC
> >>>>> hooks but would be disallowed from XDP, since there's no
> >>>>> specific plan to support this).
> >>>>
> >>>> Hmm, given skb is NULL here the only way to lookup the socket in
> >>>> such scenario is based on hash, that is, inet_ehashfn() /
> >>>> inet6_ehashfn(), perhaps alternative is to pass this hash in
> >>>> from XDP itself to the helper so it could be custom selector. Do
> >>>> you have a specific use case on this for XDP (just curious)?
> >>>
> >>> I don't have a use case for SO_REUSEPORT introspection from XDP,
> >>> so I'm primarily thinking from the perspective of making the
> >>> behaviour clear in the API in a way that leaves open the
> >>> possibility for a reasonable implementation in future. From that
> >>> perspective, my main concern is that it may surprise some BPF
> >>> writers that the same "bpf_sk_lookup_tcp()" call (with identical
> >>> parameters) may have different behaviour at TC vs. XDP layers, as
> >>> the BPF selection of sockets is respected at TC but not at XDP.
> >>>
> >>> FWIW we're already out of parameters for the actual call, so if we
> >>> wanted to allow passing a hash in, we'd need to either dedicate
> >>> half the 'flags' field for this configurable hash, or consider
> >>> adding the new hash parameter to 'struct bpf_sock_tuple'.
> >>>
> >>> +Martin for any thoughts on SO_REUSEPORT and XDP here.
> >> The XDP/TC prog has read access to the sk fields through
> >> 'struct bpf_sock'?
> >>
> >> A quick thought...
> >> Considering all sk in the same reuse->socks[] share
> >> many things (e.g. family,type,protocol,ip,port..etc are the same),
> >> I wonder returning which particular sk from reuse->socks[] will
> >> matter too much since most of the fields from 'struct bpf_sock'
> >> will be the same. Some of fields in 'struct bpf_sock' could be
> >> different though, like priority? Hence, another possibility is to
> >> limit the accessible fields for the XDP prog. Only allow
> >> accessing the fields that must be the same among the sk in the
> >> same reuse->socks[].
> >
> > This sounds pretty reasonable to me.
>
> Agree, and in any case this difference in returned sk selection should
> probably also be documented in the uapi helper description.
Okay, will do in a v2.
Thanks
Nitin
^ permalink raw reply
* Re: [bpf-next v3 0/2] Fix kcm + sockmap by checking psock type
From: Daniel Borkmann @ 2018-10-19 22:57 UTC (permalink / raw)
To: John Fastabend, ast, eric.dumazet; +Cc: netdev
In-Reply-To: <76735602-6af6-bc03-ee66-294987e768e5@iogearbox.net>
On 10/20/2018 12:51 AM, Daniel Borkmann wrote:
> On 10/18/2018 10:58 PM, John Fastabend wrote:
>> We check if the sk_user_data (the psock in skmsg) is in fact a sockmap
>> type to late, after we read the refcnt which is an error. This
>> series moves the check up before reading refcnt and also adds a test
>> to test_maps to test trying to add a KCM socket into a sockmap.
>>
>> While reviewig this code I also found an issue with KCM and kTLS
>> where each uses sk_data_ready hooks and associated stream parser
>> breaking expectations in kcm, ktls or both. But that fix will need
>> to go to net.
>>
>> Thanks to Eric for reporting.
>>
>> v2: Fix up file +/- my scripts lost track of them
>> v3: return EBUSY if refcnt is zero
>>
>> John Fastabend (2):
>> bpf: skmsg, fix psock create on existing kcm/tls port
>> bpf: test_maps add a test to catch kcm + sockmap
>>
>> include/linux/skmsg.h | 25 +++++++++---
>> net/core/sock_map.c | 11 +++---
>> tools/testing/selftests/bpf/Makefile | 2 +-
>> tools/testing/selftests/bpf/sockmap_kcm.c | 14 +++++++
>> tools/testing/selftests/bpf/test_maps.c | 64 ++++++++++++++++++++++++++++++-
>> 5 files changed, 103 insertions(+), 13 deletions(-)
>> create mode 100644 tools/testing/selftests/bpf/sockmap_kcm.c
>
> Applied, thanks!
Fyi, I've only applied patch 1/2 for now to get the bug fixed. The patch 2/2 throws
a bunch of warnings that look like the below. Also, I think we leak kcm socket in
error paths and once we're done with testing, so would be good to close it once
unneeded. Please respin the test as a stand-alone commit, thanks:
[...]
bpf-next/tools/testing/selftests/bpf/libbpf.a -lcap -lelf -lrt -lpthread -o /home/darkstar/trees/bpf-next-ok/tools/testing/selftests/bpf/test_maps
test_maps.c: In function ‘test_sockmap’:
test_maps.c:869:0: warning: "AF_KCM" redefined
#define AF_KCM 41
In file included from /usr/include/sys/socket.h:38:0,
from test_maps.c:21:
/usr/include/bits/socket.h:133:0: note: this is the location of the previous definition
#define AF_KCM PF_KCM
^ permalink raw reply
* Re: [bpf-next v3 0/2] Fix kcm + sockmap by checking psock type
From: Daniel Borkmann @ 2018-10-19 22:51 UTC (permalink / raw)
To: John Fastabend, ast, eric.dumazet; +Cc: netdev
In-Reply-To: <1539896316-13403-1-git-send-email-john.fastabend@gmail.com>
On 10/18/2018 10:58 PM, John Fastabend wrote:
> We check if the sk_user_data (the psock in skmsg) is in fact a sockmap
> type to late, after we read the refcnt which is an error. This
> series moves the check up before reading refcnt and also adds a test
> to test_maps to test trying to add a KCM socket into a sockmap.
>
> While reviewig this code I also found an issue with KCM and kTLS
> where each uses sk_data_ready hooks and associated stream parser
> breaking expectations in kcm, ktls or both. But that fix will need
> to go to net.
>
> Thanks to Eric for reporting.
>
> v2: Fix up file +/- my scripts lost track of them
> v3: return EBUSY if refcnt is zero
>
> John Fastabend (2):
> bpf: skmsg, fix psock create on existing kcm/tls port
> bpf: test_maps add a test to catch kcm + sockmap
>
> include/linux/skmsg.h | 25 +++++++++---
> net/core/sock_map.c | 11 +++---
> tools/testing/selftests/bpf/Makefile | 2 +-
> tools/testing/selftests/bpf/sockmap_kcm.c | 14 +++++++
> tools/testing/selftests/bpf/test_maps.c | 64 ++++++++++++++++++++++++++++++-
> 5 files changed, 103 insertions(+), 13 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/sockmap_kcm.c
Applied, thanks!
^ permalink raw reply
* Re: Fw: [Bug 201423] New: eth0: hw csum failure
From: Eric Dumazet @ 2018-10-19 22:25 UTC (permalink / raw)
To: Eric Dumazet, Eric Dumazet, andre
Cc: Stephen Hemminger, netdev, rossi.f, Dimitris Michailidis
In-Reply-To: <4693819f-4a76-532f-9b24-d4328183c807@gmail.com>
On 10/19/2018 02:58 PM, Eric Dumazet wrote:
>
>
> On 10/16/2018 06:00 AM, Eric Dumazet wrote:
>> On Mon, Oct 15, 2018 at 11:30 PM Andre Tomt <andre@tomt.net> wrote:
>>>
>>> On 15.10.2018 17:41, Eric Dumazet wrote:
>>>> On Mon, Oct 15, 2018 at 8:15 AM Stephen Hemminger
>>>>> Something is changed between 4.17.12 and 4.18, after bisecting the problem I
>>>>> got the following first bad commit:
>>>>>
>>>>> commit 88078d98d1bb085d72af8437707279e203524fa5
>>>>> Author: Eric Dumazet <edumazet@google.com>
>>>>> Date: Wed Apr 18 11:43:15 2018 -0700
>>>>>
>>>>> net: pskb_trim_rcsum() and CHECKSUM_COMPLETE are friends
>>>>>
>>>>> After working on IP defragmentation lately, I found that some large
>>>>> packets defeat CHECKSUM_COMPLETE optimization because of NIC adding
>>>>> zero paddings on the last (small) fragment.
>>>>>
>>>>> While removing the padding with pskb_trim_rcsum(), we set skb->ip_summed
>>>>> to CHECKSUM_NONE, forcing a full csum validation, even if all prior
>>>>> fragments had CHECKSUM_COMPLETE set.
>>>>>
>>>>> We can instead compute the checksum of the part we are trimming,
>>>>> usually smaller than the part we keep.
>>>>>
>>>>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>>>>> Signed-off-by: David S. Miller <davem@davemloft.net>
>>>>>
>>>>
>>>> Thanks for bisecting !
>>>>
>>>> This commit is known to expose some NIC/driver bugs.
>>>>
>>>> Look at commit 12b03558cef6d655d0d394f5e98a6fd07c1f6c0f
>>>> ("net: sungem: fix rx checksum support") for one driver needing a fix.
>>>>
>>>> I assume SKY2_HW_NEW_LE is not set on your NIC ?
>>>>
>>>
>>> I've seen similar on several systems with mlx4 cards when using 4.18.x -
>>> that is hw csum failure followed by some backtrace.
>>>
>>> Only seems to happen on systems dealing with quite a bit of UDP.
>>>
>>
>> Strange, because mlx4 on IPv6+UDP should not use CHECKSUM_COMPLETE,
>> but CHECKSUM_UNNECESSARY
>>
>> I would be nice to track this a bit further, maybe by providing the
>> full packet content.
>>
>>> Example from 4.18.10:
>>>> [635607.740574] p0xe0: hw csum failure
>>>> [635607.740598] CPU: 4 PID: 0 Comm: swapper/4 Not tainted 4.18.0-1 #1
>>>> [635607.740599] Hardware name: Supermicro Super Server/X10SRL-F, BIOS 2.0b 05/02/2017
>>>> [635607.740599] Call Trace:
>>>> [635607.740602] <IRQ>
>>>> [635607.740611] dump_stack+0x5c/0x7b
>>>> [635607.740617] __skb_gro_checksum_complete+0x9a/0xa0
>>>> [635607.740621] udp6_gro_receive+0x211/0x290
>>>> [635607.740624] ipv6_gro_receive+0x1a8/0x390
>>>> [635607.740627] dev_gro_receive+0x33e/0x550
>>>> [635607.740628] napi_gro_frags+0xa2/0x210
>>>> [635607.740635] mlx4_en_process_rx_cq+0xa01/0xb40 [mlx4_en]
>>>> [635607.740648] ? mlx4_cq_completion+0x23/0x70 [mlx4_core]
>>>> [635607.740654] ? mlx4_eq_int+0x373/0xc80 [mlx4_core]
>>>> [635607.740657] mlx4_en_poll_rx_cq+0x55/0xf0 [mlx4_en]
>>>> [635607.740658] net_rx_action+0xe0/0x2e0
>>>> [635607.740662] __do_softirq+0xd8/0x2e5
>>>> [635607.740666] irq_exit+0xb4/0xc0
>>>> [635607.740667] do_IRQ+0x85/0xd0
>>>> [635607.740670] common_interrupt+0xf/0xf
>>>> [635607.740671] </IRQ>
>>>> [635607.740675] RIP: 0010:cpuidle_enter_state+0xb4/0x2a0
>>>> [635607.740675] Code: 31 ff e8 df a6 ba ff 45 84 f6 74 17 9c 58 0f 1f 44 00 00 f6 c4 02 0f 85 d8 01 00 00 31 ff e8 13 81 bf ff fb 66 0f 1f 44 00 00 <4c> 29 fb 48 ba cf f7 53 e3 a5 9b c4 20 48 89 d8 48 c1 fb 3f 48 f7
>>>> [635607.740701] RSP: 0018:ffffa5c206353ea8 EFLAGS: 00000246 ORIG_RAX: ffffffffffffffd9
>>>> [635607.740703] RAX: ffff8d72ffd20f00 RBX: 00024214f597c5b0 RCX: 000000000000001f
>>>> [635607.740703] RDX: 00024214f597c5b0 RSI: 0000000000020780 RDI: 0000000000000000
>>>> [635607.740704] RBP: 0000000000000004 R08: 002542bfbefa99fa R09: 00000000ffffffff
>>>> [635607.740705] R10: ffffa5c206353e88 R11: 00000000000000c5 R12: ffffffffaf0aaf78
>>>> [635607.740706] R13: ffff8d72ffd297d8 R14: 0000000000000000 R15: 00024214f58c2ed5
>>>> [635607.740709] ? cpuidle_enter_state+0x91/0x2a0
>>>> [635607.740712] do_idle+0x1d0/0x240
>>>> [635607.740715] cpu_startup_entry+0x5f/0x70
>>>> [635607.740719] start_secondary+0x185/0x1a0
>>>> [635607.740722] secondary_startup_64+0xa5/0xb0
>>>> [635607.740731] p0xe0: hw csum failure
>>>> [635607.740745] CPU: 4 PID: 0 Comm: swapper/4 Not tainted 4.18.0-1 #1
>>>> [635607.740746] Hardware name: Supermicro Super Server/X10SRL-F, BIOS 2.0b 05/02/2017
>>>> [635607.740746] Call Trace:
>>>> [635607.740747] <IRQ>
>>>> [635607.740750] dump_stack+0x5c/0x7b
>>>> [635607.740755] __skb_checksum_complete+0xb8/0xd0
>>>> [635607.740760] __udp6_lib_rcv+0xa6b/0xa70
>>>> [635607.740767] ? nft_do_chain_inet+0x7a/0xd0 [nf_tables]
>>>> [635607.740770] ? nft_do_chain_inet+0x7a/0xd0 [nf_tables]
>>>> [635607.740774] ip6_input_finish+0xc0/0x460
>>>> [635607.740776] ip6_input+0x2b/0x90
>>>> [635607.740778] ? ip6_rcv_finish+0x110/0x110
>>>> [635607.740780] ipv6_rcv+0x2cd/0x4b0
>>>> [635607.740783] ? udp6_lib_lookup_skb+0x59/0x80
>>>> [635607.740785] __netif_receive_skb_core+0x455/0xb30
>>>> [635607.740788] ? ipv6_gro_receive+0x1a8/0x390
>>>> [635607.740790] ? netif_receive_skb_internal+0x24/0xb0
>>>> [635607.740792] netif_receive_skb_internal+0x24/0xb0
>>>> [635607.740793] napi_gro_frags+0x165/0x210
>>>> [635607.740796] mlx4_en_process_rx_cq+0xa01/0xb40 [mlx4_en]
>>>> [635607.740802] ? mlx4_cq_completion+0x23/0x70 [mlx4_core]
>>>> [635607.740807] ? mlx4_eq_int+0x373/0xc80 [mlx4_core]
>>>> [635607.740810] mlx4_en_poll_rx_cq+0x55/0xf0 [mlx4_en]
>>>> [635607.740811] net_rx_action+0xe0/0x2e0
>>>> [635607.740813] __do_softirq+0xd8/0x2e5
>>>> [635607.740816] irq_exit+0xb4/0xc0
>>>> [635607.740817] do_IRQ+0x85/0xd0
>>>> [635607.740820] common_interrupt+0xf/0xf
>>>> [635607.740821] </IRQ>
>>>> [635607.740823] RIP: 0010:cpuidle_enter_state+0xb4/0x2a0
>>>> [635607.740823] Code: 31 ff e8 df a6 ba ff 45 84 f6 74 17 9c 58 0f 1f 44 00 00 f6 c4 02 0f 85 d8 01 00 00 31 ff e8 13 81 bf ff fb 66 0f 1f 44 00 00 <4c> 29 fb 48 ba cf f7 53 e3 a5 9b c4 20 48 89 d8 48 c1 fb 3f 48 f7
>>>> [635607.740848] RSP: 0018:ffffa5c206353ea8 EFLAGS: 00000246 ORIG_RAX: ffffffffffffffd9
>>>> [635607.740849] RAX: ffff8d72ffd20f00 RBX: 00024214f597c5b0 RCX: 000000000000001f
>>>> [635607.740850] RDX: 00024214f597c5b0 RSI: 0000000000020780 RDI: 0000000000000000
>>>> [635607.740851] RBP: 0000000000000004 R08: 002542bfbefa99fa R09: 00000000ffffffff
>>>> [635607.740852] R10: ffffa5c206353e88 R11: 00000000000000c5 R12: ffffffffaf0aaf78
>>>> [635607.740853] R13: ffff8d72ffd297d8 R14: 0000000000000000 R15: 00024214f58c2ed5
>>>> [635607.740855] ? cpuidle_enter_state+0x91/0x2a0
>>>> [635607.740857] do_idle+0x1d0/0x240
>>>> [635607.740859] cpu_startup_entry+0x5f/0x70
>>>> [635607.740861] start_secondary+0x185/0x1a0
>>>> [635607.740863] secondary_startup_64+0xa5/0xb0
>
> As a matter of fact Dimitris found the issue in the patch and is working on a fix involving csum_block_sub()
>
> Problems comes from trimming an odd number of bytes.
More exactly, trimming bytes starting at an odd offset.
^ permalink raw reply
* Re: Fw: [Bug 201423] New: eth0: hw csum failure
From: Eric Dumazet @ 2018-10-19 21:58 UTC (permalink / raw)
To: Eric Dumazet, andre
Cc: Stephen Hemminger, netdev, rossi.f, Dimitris Michailidis
In-Reply-To: <CANn89i+Q2z6DZx3q4KR7sU2hY6td2B61GMtyroLmTXFqJH4XRw@mail.gmail.com>
On 10/16/2018 06:00 AM, Eric Dumazet wrote:
> On Mon, Oct 15, 2018 at 11:30 PM Andre Tomt <andre@tomt.net> wrote:
>>
>> On 15.10.2018 17:41, Eric Dumazet wrote:
>>> On Mon, Oct 15, 2018 at 8:15 AM Stephen Hemminger
>>>> Something is changed between 4.17.12 and 4.18, after bisecting the problem I
>>>> got the following first bad commit:
>>>>
>>>> commit 88078d98d1bb085d72af8437707279e203524fa5
>>>> Author: Eric Dumazet <edumazet@google.com>
>>>> Date: Wed Apr 18 11:43:15 2018 -0700
>>>>
>>>> net: pskb_trim_rcsum() and CHECKSUM_COMPLETE are friends
>>>>
>>>> After working on IP defragmentation lately, I found that some large
>>>> packets defeat CHECKSUM_COMPLETE optimization because of NIC adding
>>>> zero paddings on the last (small) fragment.
>>>>
>>>> While removing the padding with pskb_trim_rcsum(), we set skb->ip_summed
>>>> to CHECKSUM_NONE, forcing a full csum validation, even if all prior
>>>> fragments had CHECKSUM_COMPLETE set.
>>>>
>>>> We can instead compute the checksum of the part we are trimming,
>>>> usually smaller than the part we keep.
>>>>
>>>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>>>> Signed-off-by: David S. Miller <davem@davemloft.net>
>>>>
>>>
>>> Thanks for bisecting !
>>>
>>> This commit is known to expose some NIC/driver bugs.
>>>
>>> Look at commit 12b03558cef6d655d0d394f5e98a6fd07c1f6c0f
>>> ("net: sungem: fix rx checksum support") for one driver needing a fix.
>>>
>>> I assume SKY2_HW_NEW_LE is not set on your NIC ?
>>>
>>
>> I've seen similar on several systems with mlx4 cards when using 4.18.x -
>> that is hw csum failure followed by some backtrace.
>>
>> Only seems to happen on systems dealing with quite a bit of UDP.
>>
>
> Strange, because mlx4 on IPv6+UDP should not use CHECKSUM_COMPLETE,
> but CHECKSUM_UNNECESSARY
>
> I would be nice to track this a bit further, maybe by providing the
> full packet content.
>
>> Example from 4.18.10:
>>> [635607.740574] p0xe0: hw csum failure
>>> [635607.740598] CPU: 4 PID: 0 Comm: swapper/4 Not tainted 4.18.0-1 #1
>>> [635607.740599] Hardware name: Supermicro Super Server/X10SRL-F, BIOS 2.0b 05/02/2017
>>> [635607.740599] Call Trace:
>>> [635607.740602] <IRQ>
>>> [635607.740611] dump_stack+0x5c/0x7b
>>> [635607.740617] __skb_gro_checksum_complete+0x9a/0xa0
>>> [635607.740621] udp6_gro_receive+0x211/0x290
>>> [635607.740624] ipv6_gro_receive+0x1a8/0x390
>>> [635607.740627] dev_gro_receive+0x33e/0x550
>>> [635607.740628] napi_gro_frags+0xa2/0x210
>>> [635607.740635] mlx4_en_process_rx_cq+0xa01/0xb40 [mlx4_en]
>>> [635607.740648] ? mlx4_cq_completion+0x23/0x70 [mlx4_core]
>>> [635607.740654] ? mlx4_eq_int+0x373/0xc80 [mlx4_core]
>>> [635607.740657] mlx4_en_poll_rx_cq+0x55/0xf0 [mlx4_en]
>>> [635607.740658] net_rx_action+0xe0/0x2e0
>>> [635607.740662] __do_softirq+0xd8/0x2e5
>>> [635607.740666] irq_exit+0xb4/0xc0
>>> [635607.740667] do_IRQ+0x85/0xd0
>>> [635607.740670] common_interrupt+0xf/0xf
>>> [635607.740671] </IRQ>
>>> [635607.740675] RIP: 0010:cpuidle_enter_state+0xb4/0x2a0
>>> [635607.740675] Code: 31 ff e8 df a6 ba ff 45 84 f6 74 17 9c 58 0f 1f 44 00 00 f6 c4 02 0f 85 d8 01 00 00 31 ff e8 13 81 bf ff fb 66 0f 1f 44 00 00 <4c> 29 fb 48 ba cf f7 53 e3 a5 9b c4 20 48 89 d8 48 c1 fb 3f 48 f7
>>> [635607.740701] RSP: 0018:ffffa5c206353ea8 EFLAGS: 00000246 ORIG_RAX: ffffffffffffffd9
>>> [635607.740703] RAX: ffff8d72ffd20f00 RBX: 00024214f597c5b0 RCX: 000000000000001f
>>> [635607.740703] RDX: 00024214f597c5b0 RSI: 0000000000020780 RDI: 0000000000000000
>>> [635607.740704] RBP: 0000000000000004 R08: 002542bfbefa99fa R09: 00000000ffffffff
>>> [635607.740705] R10: ffffa5c206353e88 R11: 00000000000000c5 R12: ffffffffaf0aaf78
>>> [635607.740706] R13: ffff8d72ffd297d8 R14: 0000000000000000 R15: 00024214f58c2ed5
>>> [635607.740709] ? cpuidle_enter_state+0x91/0x2a0
>>> [635607.740712] do_idle+0x1d0/0x240
>>> [635607.740715] cpu_startup_entry+0x5f/0x70
>>> [635607.740719] start_secondary+0x185/0x1a0
>>> [635607.740722] secondary_startup_64+0xa5/0xb0
>>> [635607.740731] p0xe0: hw csum failure
>>> [635607.740745] CPU: 4 PID: 0 Comm: swapper/4 Not tainted 4.18.0-1 #1
>>> [635607.740746] Hardware name: Supermicro Super Server/X10SRL-F, BIOS 2.0b 05/02/2017
>>> [635607.740746] Call Trace:
>>> [635607.740747] <IRQ>
>>> [635607.740750] dump_stack+0x5c/0x7b
>>> [635607.740755] __skb_checksum_complete+0xb8/0xd0
>>> [635607.740760] __udp6_lib_rcv+0xa6b/0xa70
>>> [635607.740767] ? nft_do_chain_inet+0x7a/0xd0 [nf_tables]
>>> [635607.740770] ? nft_do_chain_inet+0x7a/0xd0 [nf_tables]
>>> [635607.740774] ip6_input_finish+0xc0/0x460
>>> [635607.740776] ip6_input+0x2b/0x90
>>> [635607.740778] ? ip6_rcv_finish+0x110/0x110
>>> [635607.740780] ipv6_rcv+0x2cd/0x4b0
>>> [635607.740783] ? udp6_lib_lookup_skb+0x59/0x80
>>> [635607.740785] __netif_receive_skb_core+0x455/0xb30
>>> [635607.740788] ? ipv6_gro_receive+0x1a8/0x390
>>> [635607.740790] ? netif_receive_skb_internal+0x24/0xb0
>>> [635607.740792] netif_receive_skb_internal+0x24/0xb0
>>> [635607.740793] napi_gro_frags+0x165/0x210
>>> [635607.740796] mlx4_en_process_rx_cq+0xa01/0xb40 [mlx4_en]
>>> [635607.740802] ? mlx4_cq_completion+0x23/0x70 [mlx4_core]
>>> [635607.740807] ? mlx4_eq_int+0x373/0xc80 [mlx4_core]
>>> [635607.740810] mlx4_en_poll_rx_cq+0x55/0xf0 [mlx4_en]
>>> [635607.740811] net_rx_action+0xe0/0x2e0
>>> [635607.740813] __do_softirq+0xd8/0x2e5
>>> [635607.740816] irq_exit+0xb4/0xc0
>>> [635607.740817] do_IRQ+0x85/0xd0
>>> [635607.740820] common_interrupt+0xf/0xf
>>> [635607.740821] </IRQ>
>>> [635607.740823] RIP: 0010:cpuidle_enter_state+0xb4/0x2a0
>>> [635607.740823] Code: 31 ff e8 df a6 ba ff 45 84 f6 74 17 9c 58 0f 1f 44 00 00 f6 c4 02 0f 85 d8 01 00 00 31 ff e8 13 81 bf ff fb 66 0f 1f 44 00 00 <4c> 29 fb 48 ba cf f7 53 e3 a5 9b c4 20 48 89 d8 48 c1 fb 3f 48 f7
>>> [635607.740848] RSP: 0018:ffffa5c206353ea8 EFLAGS: 00000246 ORIG_RAX: ffffffffffffffd9
>>> [635607.740849] RAX: ffff8d72ffd20f00 RBX: 00024214f597c5b0 RCX: 000000000000001f
>>> [635607.740850] RDX: 00024214f597c5b0 RSI: 0000000000020780 RDI: 0000000000000000
>>> [635607.740851] RBP: 0000000000000004 R08: 002542bfbefa99fa R09: 00000000ffffffff
>>> [635607.740852] R10: ffffa5c206353e88 R11: 00000000000000c5 R12: ffffffffaf0aaf78
>>> [635607.740853] R13: ffff8d72ffd297d8 R14: 0000000000000000 R15: 00024214f58c2ed5
>>> [635607.740855] ? cpuidle_enter_state+0x91/0x2a0
>>> [635607.740857] do_idle+0x1d0/0x240
>>> [635607.740859] cpu_startup_entry+0x5f/0x70
>>> [635607.740861] start_secondary+0x185/0x1a0
>>> [635607.740863] secondary_startup_64+0xa5/0xb0
As a matter of fact Dimitris found the issue in the patch and is working on a fix involving csum_block_sub()
Problems comes from trimming an odd number of bytes.
^ 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