* Re: [PATCH v2 bpf-next 0/3] bpf/verifier: subprog/func_call simplifications
From: Edward Cree @ 2018-03-29 22:50 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann; +Cc: netdev
In-Reply-To: <d13f6b60-3632-3e4d-111a-2edb4198b473@solarflare.com>
On 29/03/18 23:44, Edward Cree wrote:
> By storing subprog boundaries as a subprogno mark on each insn, rather than
> a start (and implicit end) for each subprog, we collect a number of gains:
> * More efficient determination of which subprog contains a given insn, and
> thus of find_subprog (which subprog begins at a given insn).
> * Number of verifier "full recursive walk" passes is reduced, since most of
> the work is done in the main insn walk (do_check()). Leftover work in
> other passes is mostly linear scans (O(insn_cnt)) or, in the case of
> check_max_stack_depth(), a topological sort (O(subprog_cnt)).
>
> Some other changes were also included to support this:
> * Per-subprog info is stored in env->subprog_info, an array of structs,
> rather than several arrays with a common index.
> * Call graph is now stored in the new bpf_subprog_info struct; used here
> for check_max_stack_depth() but may have other uses too.
>
> Along with this, patch #3 puts parent pointers (used by liveness analysis)
> in the registers instead of the func_state or verifier_state, so that we
> don't need skip_callee() machinery. This also does the right thing for
> stack slots, so they don't need their own special handling for liveness
> marking either.
Whoops, forgot to add:
Changes from v1:
* No longer allows non-contiguous subprogs.
* No longer allows LD_ABS|IND and pseudo-calls in the same prog.
> Edward Cree (3):
> bpf/verifier: validate func_calls by marking at do_check() time
> bpf/verifier: update selftests
> bpf/verifier: per-register parent pointers
>
> include/linux/bpf_verifier.h | 32 +-
> kernel/bpf/verifier.c | 631 +++++++++++++---------------
> tools/testing/selftests/bpf/test_verifier.c | 51 ++-
> 3 files changed, 344 insertions(+), 370 deletions(-)
>
^ permalink raw reply
* [PATCH net-next v1 0/2] Meson8m2 support for dwmac-meson8b
From: Martin Blumenstingl @ 2018-03-29 23:00 UTC (permalink / raw)
To: robh+dt, mark.rutland, netdev, linux-amlogic, davem
Cc: carlo, khilman, Martin Blumenstingl
The Meson8m2 SoC is an updated version of the Meson8 SoC. Some of the
peripherals are shared with Meson8b (for example the watchdog registers
and the internal temperature sensor calibration procedure).
Meson8m2 also seems to include the same Gigabit MAC register layout as
Meson8b.
The registers in the Amlogic dwmac "glue" seem identical between Meson8b
and Meson8m2. Manual testing seems to confirm this.
To be extra-safe a new compatible string is added because there's no
(public) documentation on the Meson8m2 SoC. This will allow us to
implement any SoC-specific variations later on (if needed).
Martin Blumenstingl (2):
dt-bindings: net: meson-dwmac: add support for the Meson8m2 SoC
net: stmmac: dwmac-meson8b: Add support for the Meson8m2 SoC
Documentation/devicetree/bindings/net/meson-dwmac.txt | 5 +++--
drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 5 +++--
2 files changed, 6 insertions(+), 4 deletions(-)
--
2.16.3
^ permalink raw reply
* [PATCH net-next v1 1/2] dt-bindings: net: meson-dwmac: add support for the Meson8m2 SoC
From: Martin Blumenstingl @ 2018-03-29 23:00 UTC (permalink / raw)
To: robh+dt, mark.rutland, netdev, linux-amlogic, davem
Cc: carlo, khilman, Martin Blumenstingl
In-Reply-To: <20180329230035.20958-1-martin.blumenstingl@googlemail.com>
The Meson8m2 SoC uses a similar (potentially even identical) register
layout for the dwmac glue as Meson8b and GXBB. Unfortunately there is no
documentation available.
Testing shows that both, RMII and RGMII PHYs are working if they are
configured as on Meson8b. Add a new compatible string to the
documentation so differences (if there are any) between Meson8m2 and the
other SoCs can be taken care of within the driver.
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
Documentation/devicetree/bindings/net/meson-dwmac.txt | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/meson-dwmac.txt b/Documentation/devicetree/bindings/net/meson-dwmac.txt
index 354dd9896bb5..61cada22ae6c 100644
--- a/Documentation/devicetree/bindings/net/meson-dwmac.txt
+++ b/Documentation/devicetree/bindings/net/meson-dwmac.txt
@@ -9,6 +9,7 @@ Required properties on all platforms:
- compatible: Depending on the platform this should be one of:
- "amlogic,meson6-dwmac"
- "amlogic,meson8b-dwmac"
+ - "amlogic,meson8m2-dwmac"
- "amlogic,meson-gxbb-dwmac"
Additionally "snps,dwmac" and any applicable more
detailed version number described in net/stmmac.txt
@@ -19,13 +20,13 @@ Required properties on all platforms:
configuration (for example the PRG_ETHERNET register range
on Meson8b and newer)
-Required properties on Meson8b and newer:
+Required properties on Meson8b, Meson8m2, GXBB and newer:
- clock-names: Should contain the following:
- "stmmaceth" - see stmmac.txt
- "clkin0" - first parent clock of the internal mux
- "clkin1" - second parent clock of the internal mux
-Optional properties on Meson8b and newer:
+Optional properties on Meson8b, Meson8m2, GXBB and newer:
- amlogic,tx-delay-ns: The internal RGMII TX clock delay (provided
by this driver) in nanoseconds. Allowed values
are: 0ns, 2ns, 4ns, 6ns.
--
2.16.3
^ permalink raw reply related
* [PATCH net-next v1 2/2] net: stmmac: dwmac-meson8b: Add support for the Meson8m2 SoC
From: Martin Blumenstingl @ 2018-03-29 23:00 UTC (permalink / raw)
To: robh+dt, mark.rutland, netdev, linux-amlogic, davem
Cc: carlo, khilman, Martin Blumenstingl
In-Reply-To: <20180329230035.20958-1-martin.blumenstingl@googlemail.com>
The Meson8m2 SoC uses a similar (potentially even identical) register
layout as the Meson8b and GXBB SoCs for the dwmac glue.
Add a new compatible string and update the module description to
indicate support for these SoCs.
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 2d5d4aea3bcb..7cb794094a70 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -1,5 +1,5 @@
/*
- * Amlogic Meson8b and GXBB DWMAC glue layer
+ * Amlogic Meson8b, Meson8m2 and GXBB DWMAC glue layer
*
* Copyright (C) 2016 Martin Blumenstingl <martin.blumenstingl@googlemail.com>
*
@@ -318,6 +318,7 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
static const struct of_device_id meson8b_dwmac_match[] = {
{ .compatible = "amlogic,meson8b-dwmac" },
+ { .compatible = "amlogic,meson8m2-dwmac" },
{ .compatible = "amlogic,meson-gxbb-dwmac" },
{ }
};
@@ -335,5 +336,5 @@ static struct platform_driver meson8b_dwmac_driver = {
module_platform_driver(meson8b_dwmac_driver);
MODULE_AUTHOR("Martin Blumenstingl <martin.blumenstingl@googlemail.com>");
-MODULE_DESCRIPTION("Amlogic Meson8b and GXBB DWMAC glue layer");
+MODULE_DESCRIPTION("Amlogic Meson8b, Meson8m2 and GXBB DWMAC glue layer");
MODULE_LICENSE("GPL v2");
--
2.16.3
^ permalink raw reply related
* Re: [PATCH v2 bpf-next 3/9] bpf: Hooks for sys_bind
From: Daniel Borkmann @ 2018-03-29 23:06 UTC (permalink / raw)
To: Alexei Starovoitov, davem; +Cc: netdev, kernel-team
In-Reply-To: <20180328034140.291484-4-ast@kernel.org>
On 03/28/2018 05:41 AM, Alexei Starovoitov wrote:
[...]
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index e8c7fad8c329..2dec266507dc 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -450,6 +450,13 @@ int inet_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
> if (addr_len < sizeof(struct sockaddr_in))
> goto out;
>
> + /* BPF prog is run before any checks are done so that if the prog
> + * changes context in a wrong way it will be caught.
> + */
> + err = BPF_CGROUP_RUN_PROG_INET4_BIND(sk, uaddr);
> + if (err)
> + goto out;
> +
Should the hook not come at the very beginning?
/* If the socket has its own bind function then use it. (RAW) */
if (sk->sk_prot->bind) {
err = sk->sk_prot->bind(sk, uaddr, addr_len);
goto out;
}
err = -EINVAL;
if (addr_len < sizeof(struct sockaddr_in))
goto out;
/* BPF prog is run before any checks are done so that if the prog
* changes context in a wrong way it will be caught.
*/
err = BPF_CGROUP_RUN_PROG_INET4_BIND(sk, uaddr);
if (err)
goto out;
E.g. when you have v4/v6 ping or raw sockets used from language runtimes
or apps, then they provide their own bind handler here in kernel, thus any
bind rewrite won't be caught for them. Shouldn't this be covered as well
and the BPF_CGROUP_RUN_PROG_INET4_BIND() come first?
> if (addr->sin_family != AF_INET) {
> /* Compatibility games : accept AF_UNSPEC (mapped to AF_INET)
> * only if s_addr is INADDR_ANY.
> diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
> index dbbe04018813..fa24e3f06ac6 100644
> --- a/net/ipv6/af_inet6.c
> +++ b/net/ipv6/af_inet6.c
> @@ -295,6 +295,13 @@ int inet6_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
> if (addr_len < SIN6_LEN_RFC2133)
> return -EINVAL;
>
> + /* BPF prog is run before any checks are done so that if the prog
> + * changes context in a wrong way it will be caught.
> + */
> + err = BPF_CGROUP_RUN_PROG_INET6_BIND(sk, uaddr);
> + if (err)
> + return err;
> +
> if (addr->sin6_family != AF_INET6)
> return -EAFNOSUPPORT;
(Same here?)
^ permalink raw reply
* Re: [PATCH v2 bpf-next 3/9] bpf: Hooks for sys_bind
From: Alexei Starovoitov @ 2018-03-30 0:01 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov, Andrey Ignatov; +Cc: netdev, kernel-team
In-Reply-To: <a2d286fb-a517-f728-752e-92fde87f1d77@iogearbox.net>
On 3/29/18 4:06 PM, Daniel Borkmann wrote:
> On 03/28/2018 05:41 AM, Alexei Starovoitov wrote:
> [...]
>> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
>> index e8c7fad8c329..2dec266507dc 100644
>> --- a/net/ipv4/af_inet.c
>> +++ b/net/ipv4/af_inet.c
>> @@ -450,6 +450,13 @@ int inet_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
>> if (addr_len < sizeof(struct sockaddr_in))
>> goto out;
>>
>> + /* BPF prog is run before any checks are done so that if the prog
>> + * changes context in a wrong way it will be caught.
>> + */
>> + err = BPF_CGROUP_RUN_PROG_INET4_BIND(sk, uaddr);
>> + if (err)
>> + goto out;
>> +
>
> Should the hook not come at the very beginning?
>
> /* If the socket has its own bind function then use it. (RAW) */
> if (sk->sk_prot->bind) {
> err = sk->sk_prot->bind(sk, uaddr, addr_len);
> goto out;
> }
> err = -EINVAL;
> if (addr_len < sizeof(struct sockaddr_in))
> goto out;
>
> /* BPF prog is run before any checks are done so that if the prog
> * changes context in a wrong way it will be caught.
> */
> err = BPF_CGROUP_RUN_PROG_INET4_BIND(sk, uaddr);
> if (err)
> goto out;
>
> E.g. when you have v4/v6 ping or raw sockets used from language runtimes
> or apps, then they provide their own bind handler here in kernel, thus any
> bind rewrite won't be caught for them. Shouldn't this be covered as well
> and the BPF_CGROUP_RUN_PROG_INET4_BIND() come first?
the reason for hook to be called after
'if (addr_len < sizeof(struct sockaddr_in))' check is that
'struct bpf_sock_addr' rewrite assumes either sockaddr_in
or sockaddr_in6 when accessing fields.
For example, raw_bind(s) have a variety of sockaddr_* types that
we cannot recognize from bpf side without introducing special
ctx rewriter for each possible protocol and different bpf ctx for each.
That's why the hooks are called INET4_BIND and INET6_BIND and
later in __cgroup_bpf_run_filter_sock_addr() we do:
if (sk->sk_family != AF_INET && sk->sk_family != AF_INET6)
return 0;
I don't think it's possible to have one generic bind hook
for all sk_proto. What do we pass into bpf prog as context?
They all have different sockaddr*.
Consider sockaddr_sco vs sockaddr_can, etc.
In the future this feature can be extend with per-protocol bind hooks
(if really necessary), but the hooks probably will be inside specific
raw_bind() functions instead of here.
The crazy alternative approach would be to pass blob of bytes into
bpf prog as ctx and let program parse it differently depending
on protocol, but then we'd need to make 'struct bpf_sock_addr'
variable length or size it up to the largest possible sockaddr_*.
Sanitizing fields becomes complex and so on. That won't be clean.
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: sockmap: initialize sg table entries properly
From: Prashant Bhole @ 2018-03-30 0:20 UTC (permalink / raw)
To: Daniel Borkmann
Cc: John Fastabend, Alexei Starovoitov, David S . Miller, netdev
In-Reply-To: <b1973509-ac92-504d-3cd6-603450e744f3@iogearbox.net>
On 3/28/2018 5:51 PM, Daniel Borkmann wrote:
> On 03/28/2018 08:18 AM, Prashant Bhole wrote:
>> On 3/27/2018 6:05 PM, Daniel Borkmann wrote:
>>> On 03/27/2018 10:41 AM, Prashant Bhole wrote:
>>>> On 3/27/2018 12:15 PM, John Fastabend wrote:
>>>>> On 03/25/2018 11:54 PM, Prashant Bhole wrote:
>>>>>> When CONFIG_DEBUG_SG is set, sg->sg_magic is initialized to SG_MAGIC,
>>>>>> when sg table is initialized using sg_init_table(). Magic is checked
>>>>>> while navigating the scatterlist. We hit BUG_ON when magic check is
>>>>>> failed.
>>>>>>
>>>>>> Fixed following things:
>>>>>> - Initialization of sg table in bpf_tcp_sendpage() was missing,
>>>>>> initialized it using sg_init_table()
>>>>>>
>>>>>> - bpf_tcp_sendmsg() initializes sg table using sg_init_table() before
>>>>>> entering the loop, but further consumed sg entries are initialized
>>>>>> using memset. Fixed it by replacing memset with sg_init_table() in
>>>>>> function bpf_tcp_push()
>>>>>>
>>>>>> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
>>>>>> ---
>>>>>> kernel/bpf/sockmap.c | 11 +++++++----
>>>>>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
>>>>>> index 69c5bccabd22..8a848a99d768 100644
>>>>>> --- a/kernel/bpf/sockmap.c
>>>>>> +++ b/kernel/bpf/sockmap.c
>>>>>> @@ -312,7 +312,7 @@ static int bpf_tcp_push(struct sock *sk, int apply_bytes,
>>>>>> md->sg_start++;
>>>>>> if (md->sg_start == MAX_SKB_FRAGS)
>>>>>> md->sg_start = 0;
>>>>>> - memset(sg, 0, sizeof(*sg));
>>>>>> + sg_init_table(sg, 1);
>>>>>
>>>>> Looks OK here.
>>>>>
>>>>>> if (md->sg_start == md->sg_end)
>>>>>> break;
>>>>>> @@ -763,10 +763,14 @@ static int bpf_tcp_sendpage(struct sock *sk, struct page *page,
>>>>>> lock_sock(sk);
>>>>>> - if (psock->cork_bytes)
>>>>>> + if (psock->cork_bytes) {
>>>>>> m = psock->cork;
>>>>>> - else
>>>>>> + sg = &m->sg_data[m->sg_end];
>>>>>> + } else {
>>>>>> m = &md;
>>>>>> + sg = m->sg_data;
>>>>>> + sg_init_table(sg, MAX_SKB_FRAGS);
>>>>>
>>>>> sg_init_table() does an unnecessary memset() though. We
>>>>> probably either want a new scatterlist API or just open
>>>>> code this,
>>>>>
>>>>> #ifdef CONFIG_DEBUG_SG
>>>>> {
>>>>> unsigned int i;
>>>>> for (i = 0; i < nents; i++)
>>>>> sgl[i].sg_magic = SG_MAGIC;
>>>>> }
>>>>
>>>> Similar sg_init_table() is present in bpf_tcp_sendmsg().
>>>> I agree that it causes unnecessary memset, but I don't agree with open coded fix.
>>>
>>> But then lets fix is properly and add a static inline helper to the
>>> include/linux/scatterlist.h header like ...
>>>
>>> static inline void sg_init_debug_marker(struct scatterlist *sgl,
>>> unsigned int nents)
>>> {
>>> #ifdef CONFIG_DEBUG_SG
>>> unsigned int i;
>>>
>>> for (i = 0; i < nents; i++)
>>> sgl[i].sg_magic = SG_MAGIC;
>>> #endif
>>> }
>>>
>>> ... and reuse it in all the places that would otherwise open-code this,
>>> as well as sg_init_table():
>>>
>>> void sg_init_table(struct scatterlist *sgl, unsigned int nents)
>>> {
>>> memset(sgl, 0, sizeof(*sgl) * nents);
>>> sg_init_debug_marker(sgl, nents);
>>> sg_mark_end(&sgl[nents - 1]);
>>> }
>>>
>>> This would be a lot cleaner than having this duplicated in various places.
>>
>> Daniel, This is a good suggestion. Is it ok if I submit both changes in
>> a patch series?
>
> Sure, that's fine.
>
>> How scatterlist related changes will be picked up by other subsystems?
>
> Once this gets applied into bpf-next, this will be pushed to net-next tree,
> and during the merge window net-next will be pulled into Linus' tree if this
> is what you are asking. Then also other subsystems outside of bpf/networking
> can make use of the sg_init_debug_marker() helper if suitable for their
> situation.
Thanks. I am submitting V2 soon.
-Prashant
^ permalink raw reply
* [PATCH v2 bpf-next 0/2] sockmap: fix sg api usage
From: Prashant Bhole @ 2018-03-30 0:20 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov, David S . Miller
Cc: Prashant Bhole, John Fastabend, netdev
These patches fix sg api usage in sockmap. Previously sockmap didn't
use sg_init_table(), which caused hitting BUG_ON in sg api, when
CONFIG_DEBUG_SG is enabled
v1: added sg_init_table() calls wherever needed.
v2:
- Patch1 adds new helper function in sg api. sg_init_marker()
- Patch2 sg_init_marker() and sg_init_table() in appropriate places
Backgroud:
While reviewing v1, John Fastabend raised a valid point about
unnecessary memset in sg_init_table() because sockmap uses sg table
which embedded in a struct. As enclosing struct is zeroed out, there
is unnecessary memset in sg_init_table.
So Daniel Borkmann suggested to define another static inline function
in scatterlist.h which only initializes sg_magic. Also this function
will be called from sg_init_table. From this suggestion I defined a
function sg_init_marker() which sets sg_magic and calls sg_mark_end()
Prashant Bhole (2):
lib/scatterlist: add sg_init_marker() helper
bpf: sockmap: initialize sg table entries properly
include/linux/scatterlist.h | 18 ++++++++++++++++++
kernel/bpf/sockmap.c | 13 ++++++++-----
lib/scatterlist.c | 9 +--------
3 files changed, 27 insertions(+), 13 deletions(-)
--
2.14.3
^ permalink raw reply
* [PATCH v2 bpf-next 1/2] lib/scatterlist: add sg_init_marker() helper
From: Prashant Bhole @ 2018-03-30 0:20 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov, David S . Miller
Cc: Prashant Bhole, John Fastabend, netdev
In-Reply-To: <20180330002100.5724-1-bhole_prashant_q7@lab.ntt.co.jp>
sg_init_marker initializes sg_magic in the sg table and calls
sg_mark_end() on the last entry of the table. This can be useful to
avoid memset in sg_init_table() when scatterlist is already zeroed out
For example: when scatterlist is embedded inside other struct and that
container struct is zeroed out
Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
---
include/linux/scatterlist.h | 18 ++++++++++++++++++
lib/scatterlist.c | 9 +--------
2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index 22b2131bcdcd..aa5d4eb725f5 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -248,6 +248,24 @@ static inline void *sg_virt(struct scatterlist *sg)
return page_address(sg_page(sg)) + sg->offset;
}
+/**
+ * sg_init_marker - Initialize markers in sg table
+ * @sgl: The SG table
+ * @nents: Number of entries in table
+ *
+ **/
+static inline void sg_init_marker(struct scatterlist *sgl,
+ unsigned int nents)
+{
+#ifdef CONFIG_DEBUG_SG
+ unsigned int i;
+
+ for (i = 0; i < nents; i++)
+ sgl[i].sg_magic = SG_MAGIC;
+#endif
+ sg_mark_end(&sgl[nents - 1]);
+}
+
int sg_nents(struct scatterlist *sg);
int sg_nents_for_len(struct scatterlist *sg, u64 len);
struct scatterlist *sg_next(struct scatterlist *);
diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index 53728d391d3a..06dad7a072fd 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -132,14 +132,7 @@ EXPORT_SYMBOL(sg_last);
void sg_init_table(struct scatterlist *sgl, unsigned int nents)
{
memset(sgl, 0, sizeof(*sgl) * nents);
-#ifdef CONFIG_DEBUG_SG
- {
- unsigned int i;
- for (i = 0; i < nents; i++)
- sgl[i].sg_magic = SG_MAGIC;
- }
-#endif
- sg_mark_end(&sgl[nents - 1]);
+ sg_init_marker(sgl, nents);
}
EXPORT_SYMBOL(sg_init_table);
--
2.14.3
^ permalink raw reply related
* Re: Regression in 4.16-rc7 - ipsec vpn broken
From: Derek Robson @ 2018-03-30 0:22 UTC (permalink / raw)
To: Steffen Klassert; +Cc: Herbert Xu, ben, linux-kernel, netdev
In-Reply-To: <20180329065127.yn6sm3xvs2givubo@gauss3.secunet.de>
Thanks, that patch has solved issue.
On Thu, Mar 29, 2018 at 7:51 PM, Steffen Klassert
<steffen.klassert@secunet.com> wrote:
> Please always make sure to Cc netdev@vger.kernel.org
> on networking problems.
>
> On Wed, Mar 28, 2018 at 10:21:32PM +0000, Derek Robson wrote:
>> The ipsec VPN is broken in 4.16-rc7 and seem to have been broken in all of
>> 4.15
>>
>> connecting from an iphone seems to give a timeout.
>>
>>
>> A bisect brings me to this commit as the one that is the issue.
>>
>> commit: acf568ee859f098279eadf551612f103afdacb4e (xfrm: Reinject
>> transport-mode packets through tasklet)
>
> I have a fix queued for this commit in the ipsec tree.
>
> Can you please try if the patch below fixes your problems?
>
> Thanks!
>
> Subject: [PATCH] xfrm: Fix transport mode skb control buffer usage.
>
> A recent commit introduced a new struct xfrm_trans_cb
> that is used with the sk_buff control buffer. Unfortunately
> it placed the structure in front of the control buffer and
> overlooked that the IPv4/IPv6 control buffer is still needed
> for some layer 4 protocols. As a result the IPv4/IPv6 control
> buffer is overwritten with this structure. Fix this by setting
> a apropriate header in front of the structure.
>
> Fixes acf568ee859f ("xfrm: Reinject transport-mode packets ...")
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> ---
> net/xfrm/xfrm_input.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
> index 1472c0857975..81788105c164 100644
> --- a/net/xfrm/xfrm_input.c
> +++ b/net/xfrm/xfrm_input.c
> @@ -26,6 +26,12 @@ struct xfrm_trans_tasklet {
> };
>
> struct xfrm_trans_cb {
> + union {
> + struct inet_skb_parm h4;
> +#if IS_ENABLED(CONFIG_IPV6)
> + struct inet6_skb_parm h6;
> +#endif
> + } header;
> int (*finish)(struct net *net, struct sock *sk, struct sk_buff *skb);
> };
>
> --
> 2.14.1
>
^ permalink raw reply
* [PATCH v2 bpf-next 2/2] bpf: sockmap: initialize sg table entries properly
From: Prashant Bhole @ 2018-03-30 0:21 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov, David S . Miller
Cc: Prashant Bhole, John Fastabend, netdev
In-Reply-To: <20180330002100.5724-1-bhole_prashant_q7@lab.ntt.co.jp>
When CONFIG_DEBUG_SG is set, sg->sg_magic is initialized in
sg_init_table() and it is verified in sg api while navigating. We hit
BUG_ON when magic check is failed.
In functions sg_tcp_sendpage and sg_tcp_sendmsg, the struct containing
the scatterlist is already zeroed out. So to avoid extra memset, we
use sg_init_marker() to initialize sg_magic.
Fixed following things:
- In bpf_tcp_sendpage: initialize sg using sg_init_marker
- In bpf_tcp_sendmsg: Replace sg_init_table with sg_init_marker
- In bpf_tcp_push: Replace memset with sg_init_table where consumed
sg entry needs to be re-initialized.
Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
---
kernel/bpf/sockmap.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 69c5bccabd22..b4f01656c452 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -312,7 +312,7 @@ static int bpf_tcp_push(struct sock *sk, int apply_bytes,
md->sg_start++;
if (md->sg_start == MAX_SKB_FRAGS)
md->sg_start = 0;
- memset(sg, 0, sizeof(*sg));
+ sg_init_table(sg, 1);
if (md->sg_start == md->sg_end)
break;
@@ -656,7 +656,7 @@ static int bpf_tcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
}
sg = md.sg_data;
- sg_init_table(sg, MAX_SKB_FRAGS);
+ sg_init_marker(sg, MAX_SKB_FRAGS);
rcu_read_unlock();
lock_sock(sk);
@@ -763,10 +763,14 @@ static int bpf_tcp_sendpage(struct sock *sk, struct page *page,
lock_sock(sk);
- if (psock->cork_bytes)
+ if (psock->cork_bytes) {
m = psock->cork;
- else
+ sg = &m->sg_data[m->sg_end];
+ } else {
m = &md;
+ sg = m->sg_data;
+ sg_init_marker(sg, MAX_SKB_FRAGS);
+ }
/* Catch case where ring is full and sendpage is stalled. */
if (unlikely(m->sg_end == m->sg_start &&
@@ -774,7 +778,6 @@ static int bpf_tcp_sendpage(struct sock *sk, struct page *page,
goto out_err;
psock->sg_size += size;
- sg = &m->sg_data[m->sg_end];
sg_set_page(sg, page, size, offset);
get_page(page);
m->sg_copy[m->sg_end] = true;
--
2.14.3
^ permalink raw reply related
* Re: [PATCH net-next] net/mlx4_en: CHECKSUM_COMPLETE support for fragments
From: Saeed Mahameed @ 2018-03-30 0:34 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, netdev, Eric Dumazet, Willem de Bruijn,
Tariq Toukan
In-Reply-To: <20180327212114.164202-1-edumazet@google.com>
On Tue, Mar 27, 2018 at 2:21 PM, Eric Dumazet <edumazet@google.com> wrote:
> Refine the RX check summing handling to propagate the
> hardware provided checksum so that we do not have to
> compute it later in software.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Willem de Bruijn <willemb@google.com>
> Cc: Tariq Toukan <tariqt@mellanox.com>
> ---
> drivers/net/ethernet/mellanox/mlx4/en_rx.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> index 05787efef492b1c0c6ce540ef73647fad91ce282..5c613c6663da51a4ae792eeb4d8956b54655786b 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> @@ -821,14 +821,12 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
> skb_record_rx_queue(skb, cq_ring);
>
> if (likely(dev->features & NETIF_F_RXCSUM)) {
> - if (cqe->status & cpu_to_be16(MLX4_CQE_STATUS_TCP |
> - MLX4_CQE_STATUS_UDP)) {
> + if ((cqe->status & cpu_to_be16(MLX4_CQE_STATUS_TCP |
> + MLX4_CQE_STATUS_UDP)) &&
> + (cqe->status & cpu_to_be16(MLX4_CQE_STATUS_IPOK)) &&
> + cqe->checksum == cpu_to_be16(0xffff)) {
> bool l2_tunnel;
>
LGTM, this code even aligns better with the mlx4 HW documentation:
"When L4_CSUM field is not supported, L4 checksum for TCP/UDP packets
can be validated by: (IP_OK && (TCP || UDP)) && (checksum ==
0xFFFF))."
in the code we don't even consider L4_CSUM at the moment, As a future
patch, it could be a nice acceleration for the above 3 steps
condition.
Small comment, if we expect that cqe->checksum is NOT likely to be
0xffff for UDP/TCP packets, maybe it is better performance wise to
move (cqe->checksum == cpu_to_be16(0xffff)) to be evaluated first in
the condition.
> - if (!((cqe->status & cpu_to_be16(MLX4_CQE_STATUS_IPOK)) &&
> - cqe->checksum == cpu_to_be16(0xffff)))
> - goto csum_none;
> -
> l2_tunnel = (dev->hw_enc_features & NETIF_F_RXCSUM) &&
> (cqe->vlan_my_qpn & cpu_to_be32(MLX4_CQE_L2_TUNNEL));
> ip_summed = CHECKSUM_UNNECESSARY;
> --
> 2.17.0.rc1.321.gba9d0f2565-goog
>
^ permalink raw reply
* Re: [PATCH net-next] net/mlx4_en: CHECKSUM_COMPLETE support for fragments
From: Saeed Mahameed @ 2018-03-30 0:44 UTC (permalink / raw)
To: David Miller
Cc: Eric Dumazet, Linux Netdev List, Eric Dumazet, Willem de Bruijn,
Tariq Toukan
In-Reply-To: <20180329.140727.1144450119035230504.davem@davemloft.net>
On Thu, Mar 29, 2018 at 11:07 AM, David Miller <davem@davemloft.net> wrote:
> From: Eric Dumazet <edumazet@google.com>
> Date: Tue, 27 Mar 2018 14:21:14 -0700
>
>> Refine the RX check summing handling to propagate the
>> hardware provided checksum so that we do not have to
>> compute it later in software.
>>
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>
> Tariq, please review.
Hi Dave, Eric.
The patch looks ok but i would let tariq review it and decide if he
wants to run full regression coverage on it
since it changes the default behavior of the driver's checksum reporting.
It is already weekend for him and for the team in Israel, and i don't
think this can be handled before next week :).
So it is really up to you guys.
Thanks,
Saeed.
^ permalink raw reply
* [PATCH net] net/ipv6: Fix route leaking between VRFs
From: David Ahern @ 2018-03-30 0:44 UTC (permalink / raw)
To: netdev; +Cc: sharpd, David Ahern
Donald reported that IPv6 route leaking between VRFs is not working.
The root cause is the strict argument in the call to rt6_lookup when
validating the nexthop spec.
ip6_route_check_nh validates the gateway and device (if given) of a
route spec. It in turn could call rt6_lookup (e.g., lookup in a given
table did not succeed so it falls back to a full lookup) and if so
sets the strict argument to 1. That means if the egress device is given,
the route lookup needs to return a result with the same device. This
strict requirement does not work with VRFs (IPv4 or IPv6) because the
oif in the flow struct is overridden with the index of the VRF device
to trigger a match on the l3mdev rule and force the lookup to its table.
The right long term solution is to add an l3mdev index to the flow
struct such that the oif is not overridden. That solution will not
backport well, so this patch aims for a simpler solution to relax the
strict argument if the route spec device is an l3mdev slave. As done
in other places, use the FLOWI_FLAG_SKIP_NH_OIF to know that the
RT6_LOOKUP_F_IFACE flag needs to be removed.
Reported-by: Donald Sharp <sharpd@cumulusnetworks.com>
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/ipv6/route.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index b33d057ac5eb..fc74352fac12 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -919,6 +919,9 @@ static struct rt6_info *ip6_pol_route_lookup(struct net *net,
struct rt6_info *rt, *rt_cache;
struct fib6_node *fn;
+ if (fl6->flowi6_flags & FLOWI_FLAG_SKIP_NH_OIF)
+ flags &= ~RT6_LOOKUP_F_IFACE;
+
rcu_read_lock();
fn = fib6_lookup(&table->tb6_root, &fl6->daddr, &fl6->saddr);
restart:
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net-next] bridge: Allow max MTU when multiple VLANs present
From: Toshiaki Makita @ 2018-03-30 1:02 UTC (permalink / raw)
To: Roopa Prabhu, Chas Williams
Cc: David Miller, netdev, Stephen Hemminger, Nikolay Aleksandrov
In-Reply-To: <CAJieiUiyB3rreCvnoFAzsKRFOaQdTJ3uDsEWcV2oB2Jzry2hiA@mail.gmail.com>
On 2018/03/30 1:49, Roopa Prabhu wrote:
> On Thu, Mar 22, 2018 at 9:53 PM, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>> On Thu, Mar 22, 2018 at 8:34 AM, Chas Williams <3chas3@gmail.com> wrote:
>>> If the bridge is allowing multiple VLANs, some VLANs may have
>>> different MTUs. Instead of choosing the minimum MTU for the
>>> bridge interface, choose the maximum MTU of the bridge members.
>>> With this the user only needs to set a larger MTU on the member
>>> ports that are participating in the large MTU VLANS.
>>>
>>> Signed-off-by: Chas Williams <3chas3@gmail.com>
>>> ---
>>
>> Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> This or an equivalent fix is necessary: as stated above, today the
>> bridge mtu capped at min port mtu limits all
>> vlan devices on top of the vlan filtering bridge to min port mtu.
>
>
> On further thought, since this patch changes default behavior, it may
> upset people. ie with this patch, a vlan device
> on the bridge by default will now use the bridge max mtu and that
> could cause unexpected drops in the bridge driver
> if the xmit port had a lower mtu. This may surprise users.
>
> The other equivalent fix i was thinking about is to keep the default
> behavior as is, and allow a max mtu to be
> configured on the bridge. This will allow a sys admin to fix the
> current mtu limitations if
> deployments require it.
>
> we will submit an incremental patch to re-work this patch to restore
> default behavior.
+1
This makes sense to me.
--
Toshiaki Makita
^ permalink raw reply
* Re: [PATCH net] net/ipv6: Fix route leaking between VRFs
From: David Ahern @ 2018-03-30 1:29 UTC (permalink / raw)
To: netdev; +Cc: sharpd, David Miller
In-Reply-To: <20180330004457.814-1-dsahern@gmail.com>
On 3/29/18 6:44 PM, David Ahern wrote:
> Donald reported that IPv6 route leaking between VRFs is not working.
> The root cause is the strict argument in the call to rt6_lookup when
> validating the nexthop spec.
>
> ip6_route_check_nh validates the gateway and device (if given) of a
> route spec. It in turn could call rt6_lookup (e.g., lookup in a given
> table did not succeed so it falls back to a full lookup) and if so
> sets the strict argument to 1. That means if the egress device is given,
> the route lookup needs to return a result with the same device. This
> strict requirement does not work with VRFs (IPv4 or IPv6) because the
> oif in the flow struct is overridden with the index of the VRF device
> to trigger a match on the l3mdev rule and force the lookup to its table.
>
> The right long term solution is to add an l3mdev index to the flow
> struct such that the oif is not overridden. That solution will not
> backport well, so this patch aims for a simpler solution to relax the
> strict argument if the route spec device is an l3mdev slave. As done
> in other places, use the FLOWI_FLAG_SKIP_NH_OIF to know that the
> RT6_LOOKUP_F_IFACE flag needs to be removed.
>
Forgot the fixes tag:
Fixes: ca254490c8df ("net: Add VRF support to IPv6 stack")
Dave: I can resend if needed. Key backports are to 4.14 and 4.9. Those
are the only LTS releases affected.
> Reported-by: Donald Sharp <sharpd@cumulusnetworks.com>
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
> net/ipv6/route.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index b33d057ac5eb..fc74352fac12 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -919,6 +919,9 @@ static struct rt6_info *ip6_pol_route_lookup(struct net *net,
> struct rt6_info *rt, *rt_cache;
> struct fib6_node *fn;
>
> + if (fl6->flowi6_flags & FLOWI_FLAG_SKIP_NH_OIF)
> + flags &= ~RT6_LOOKUP_F_IFACE;
> +
> rcu_read_lock();
> fn = fib6_lookup(&table->tb6_root, &fl6->daddr, &fl6->saddr);
> restart:
>
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: Benjamin Herrenschmidt @ 2018-03-30 1:40 UTC (permalink / raw)
To: Sinan Kaya, David Miller
Cc: torvalds, alexander.duyck, will.deacon, arnd, jgg, David.Laight,
oohall, linuxppc-dev, linux-rdma, alexander.h.duyck, paulmck,
netdev, linus971
In-Reply-To: <29fe17e0-9978-dc43-d02c-de8fabdc66c2@codeaurora.org>
On Thu, 2018-03-29 at 09:56 -0400, Sinan Kaya wrote:
> On 3/28/2018 11:55 AM, David Miller wrote:
> > From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Date: Thu, 29 Mar 2018 02:13:16 +1100
> >
> > > Let's fix all archs, it's way easier than fixing all drivers. Half of
> > > the archs are unused or dead anyway.
> >
> > Agreed.
> >
>
> I pinged most of the maintainers yesterday.
> Which arches do we care about these days?
> I have not been paying attention any other architecture besides arm64.
Thanks for going through that exercise !
Once sparc, s390, microblaze and mips reply, I think we'll have a good
coverage, maybe riscv is to put in that lot too.
Cheers,
Ben.
>
> arch status detail
> ------ ------------- ------------------------------------
> alpha question sent
> arc question sent ysato@users.sourceforge.jp will fix it.
> arm no issues
> arm64 no issues
> blackfin question sent about to be removed
> c6x question sent
> cris question sent
> frv
> h8300 question sent
> hexagon question sent
> ia64 no issues confirmed by Tony Luck
> m32r
> m68k question sent
> metag
> microblaze question sent
> mips question sent
> mn10300 question sent
> nios2 question sent
> openrisc no issues shorne@gmail.com says should no issues
> parisc no issues grantgrundler@gmail.com says most probably no problem but still looking
> powerpc no issues
> riscv question sent
> s390 question sent
> score question sent
> sh question sent
> sparc question sent
> tile question sent
> unicore32 question sent
> x86 no issues
> xtensa question sent
>
>
^ permalink raw reply
* [PATCH net] vlan: also check phy_driver ts_info for vlan's real device
From: Hangbin Liu @ 2018-03-30 1:44 UTC (permalink / raw)
To: netdev; +Cc: Richard Cochran, David S. Miller, Hangbin Liu
Just like function ethtool_get_ts_info(), we should also consider the
phy_driver ts_info call back. For example, driver dp83640.
Fixes: 37dd9255b2f6 ("vlan: Pass ethtool get_ts_info queries to real device.")
Acked-by: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
net/8021q/vlan_dev.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index f7e83f6..236452e 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -29,6 +29,7 @@
#include <linux/net_tstamp.h>
#include <linux/etherdevice.h>
#include <linux/ethtool.h>
+#include <linux/phy.h>
#include <net/arp.h>
#include <net/switchdev.h>
@@ -665,8 +666,11 @@ static int vlan_ethtool_get_ts_info(struct net_device *dev,
{
const struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
const struct ethtool_ops *ops = vlan->real_dev->ethtool_ops;
+ struct phy_device *phydev = vlan->real_dev->phydev;
- if (ops->get_ts_info) {
+ if (phydev && phydev->drv && phydev->drv->ts_info) {
+ return phydev->drv->ts_info(phydev, info);
+ } else if (ops->get_ts_info) {
return ops->get_ts_info(vlan->real_dev, info);
} else {
info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE |
--
2.5.5
^ permalink raw reply related
* Re: [PATCH net-next 0/6] rxrpc: Fixes
From: David Miller @ 2018-03-30 1:48 UTC (permalink / raw)
To: dhowells; +Cc: netdev, linux-afs, linux-kernel
In-Reply-To: <30571.1522362346@warthog.procyon.org.uk>
From: David Howells <dhowells@redhat.com>
Date: Thu, 29 Mar 2018 23:25:46 +0100
> David Miller <davem@davemloft.net> wrote:
>
>> David, this GIT URL has tons of unrelated changes. It seems to bring in
>> the parts of Linus's tree that haven't proagated to 'net' yet.
>
> Sorry about that, I rebased on the wrong branch by accident.
>
> I've got some more fixes. Should I just give the lot to you to pull into your
> net-next tree, given that the merge window may well open Sunday?
That's up to you.
^ permalink raw reply
* Re: [RFC PATCH V2 8/8] vhost: event suppression for packed ring
From: Tiwei Bie @ 2018-03-30 2:05 UTC (permalink / raw)
To: Jason Wang; +Cc: mst, virtualization, kvm, netdev, linux-kernel
In-Reply-To: <1522035533-11786-9-git-send-email-jasowang@redhat.com>
On Mon, Mar 26, 2018 at 11:38:53AM +0800, Jason Wang wrote:
> This patch introduces basic support for event suppression aka driver
> and device area. Compile tested only.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
[...]
> +
> +static bool vhost_notify_packed(struct vhost_dev *dev,
> + struct vhost_virtqueue *vq)
> +{
> + __virtio16 event_off_wrap, event_flags;
> + __u16 old, new;
> + bool v, wrap;
> + int off;
> +
> + /* Flush out used descriptors updates. This is paired
> + * with the barrier that the Guest executes when enabling
> + * interrupts.
> + */
> + smp_mb();
> +
> + if (vhost_get_avail(vq, event_flags,
> + &vq->driver_event->desc_event_flags) < 0) {
> + vq_err(vq, "Failed to get driver desc_event_flags");
> + return true;
> + }
> +
> + if (!(event_flags & cpu_to_vhost16(vq, RING_EVENT_FLAGS_DESC)))
> + return event_flags ==
> + cpu_to_vhost16(vq, RING_EVENT_FLAGS_ENABLE);
Maybe it would be better to not use '&' here. Because these flags
are not defined as bits which can be ORed or ANDed. Instead, they
are defined as values:
0x0 enable
0x1 disable
0x2 desc
0x3 reserved
> +
> + /* Read desc event flags before event_off and event_wrap */
> + smp_rmb();
> +
> + if (vhost_get_avail(vq, event_off_wrap,
> + &vq->driver_event->desc_event_off_warp) < 0) {
> + vq_err(vq, "Failed to get driver desc_event_off/wrap");
> + return true;
> + }
> +
> + off = vhost16_to_cpu(vq, event_off_wrap);
> +
> + wrap = off & 0x1;
> + off >>= 1;
Based on the below definitions in spec, wrap counter is
the most significant bit.
struct pvirtq_event_suppress {
le16 {
desc_event_off : 15; /* Descriptor Ring Change Event Offset */
desc_event_wrap : 1; /* Descriptor Ring Change Event Wrap Counter */
} desc; /* If desc_event_flags set to RING_EVENT_FLAGS_DESC */
le16 {
desc_event_flags : 2, /* Descriptor Ring Change Event Flags */
reserved : 14; /* Reserved, set to 0 */
} flags;
};
> +
> +
> + old = vq->signalled_used;
> + v = vq->signalled_used_valid;
> + new = vq->signalled_used = vq->last_used_idx;
> + vq->signalled_used_valid = true;
> +
> + if (unlikely(!v))
> + return true;
> +
> + return vhost_vring_packed_need_event(vq, new, old, off) &&
> + wrap == vq->used_wrap_counter;
> +}
> +
> +static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
> +{
> + if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED))
> + return vhost_notify_packed(dev, vq);
> + else
> + return vhost_notify_split(dev, vq);
> +}
> +
> /* This actually signals the guest, using eventfd. */
> void vhost_signal(struct vhost_dev *dev, struct vhost_virtqueue *vq)
> {
> @@ -2789,7 +2911,17 @@ static bool vhost_enable_notify_packed(struct vhost_dev *dev,
> __virtio16 flags;
> int ret;
>
> - /* FIXME: disable notification through device area */
> + if (!(vq->used_flags & VRING_USED_F_NO_NOTIFY))
> + return false;
> + vq->used_flags &= ~VRING_USED_F_NO_NOTIFY;
> +
> + flags = cpu_to_vhost16(vq, RING_EVENT_FLAGS_ENABLE);
> + ret = vhost_update_device_flags(vq, flags);
> + if (ret) {
> + vq_err(vq, "Failed to enable notification at %p: %d\n",
> + &vq->device_event->desc_event_flags, ret);
> + return false;
> + }
>
> /* They could have slipped one in as we were doing that: make
> * sure it's written, then check again. */
> @@ -2855,7 +2987,18 @@ EXPORT_SYMBOL_GPL(vhost_enable_notify);
> static void vhost_disable_notify_packed(struct vhost_dev *dev,
> struct vhost_virtqueue *vq)
> {
> - /* FIXME: disable notification through device area */
> + __virtio16 flags;
> + int r;
> +
> + if (vq->used_flags & VRING_USED_F_NO_NOTIFY)
> + return;
> + vq->used_flags |= VRING_USED_F_NO_NOTIFY;
> +
> + flags = cpu_to_vhost16(vq, RING_EVENT_FLAGS_DISABLE);
> + r = vhost_update_device_flags(vq, flags);
> + if (r)
> + vq_err(vq, "Failed to enable notification at %p: %d\n",
> + &vq->device_event->desc_event_flags, r);
> }
>
> static void vhost_disable_notify_split(struct vhost_dev *dev,
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 8a9df4f..02d7a36 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -96,8 +96,14 @@ struct vhost_virtqueue {
> struct vring_desc __user *desc;
> struct vring_desc_packed __user *desc_packed;
Do you think it'd be better to name the desc type as
struct vring_packed_desc? And it will be consistent
with other names, like:
struct vring_packed;
struct vring_packed_desc_event;
> };
> - struct vring_avail __user *avail;
> - struct vring_used __user *used;
> + union {
> + struct vring_avail __user *avail;
> + struct vring_packed_desc_event __user *driver_event;
> + };
> + union {
> + struct vring_used __user *used;
> + struct vring_packed_desc_event __user *device_event;
> + };
> const struct vhost_umem_node *meta_iotlb[VHOST_NUM_ADDRS];
> struct file *kick;
> struct eventfd_ctx *call_ctx;
> diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
> index e297580..7cdbf06 100644
> --- a/include/uapi/linux/virtio_ring.h
> +++ b/include/uapi/linux/virtio_ring.h
> @@ -75,6 +75,25 @@ struct vring_desc_packed {
> __virtio16 flags;
> };
>
> +/* Enable events */
> +#define RING_EVENT_FLAGS_ENABLE 0x0
> +/* Disable events */
> +#define RING_EVENT_FLAGS_DISABLE 0x1
> +/*
> + * Enable events for a specific descriptor
> + * (as specified by Descriptor Ring Change Event Offset/Wrap Counter).
> + * Only valid if VIRTIO_F_RING_EVENT_IDX has been negotiated.
> + */
> +#define RING_EVENT_FLAGS_DESC 0x2
> +/* The value 0x3 is reserved */
> +
> +struct vring_packed_desc_event {
> + /* Descriptor Ring Change Event Offset and Wrap Counter */
> + __virtio16 desc_event_off_warp;
> + /* Descriptor Ring Change Event Flags */
> + __virtio16 desc_event_flags;
Do you think it'd be better to remove the prefix (desc_event_) for
the fields. And it will be consistent with other definitions, e.g.:
struct vring_packed_desc {
/* Buffer Address. */
__virtio64 addr;
/* Buffer Length. */
__virtio32 len;
/* Buffer ID. */
__virtio16 id;
/* The flags depending on descriptor type. */
__virtio16 flags;
};
> +};
> +
> /* Virtio ring descriptors: 16 bytes. These can chain together via "next". */
> struct vring_desc {
> /* Address (guest-physical). */
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH] net: sched: do not emit messages while holding spinlock
From: Li RongQing @ 2018-03-30 2:11 UTC (permalink / raw)
To: netdev
move messages emitting out of sch_tree_lock to avoid holding
this lock too long.
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
net/sched/sch_htb.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 1ea9846cc6ce..2a4ab7caf553 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -1337,6 +1337,7 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
struct nlattr *tb[TCA_HTB_MAX + 1];
struct tc_htb_opt *hopt;
u64 rate64, ceil64;
+ int warn = 0;
/* extract all subattrs from opt attr */
if (!opt)
@@ -1499,13 +1500,11 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
cl->quantum = min_t(u64, quantum, INT_MAX);
if (!hopt->quantum && cl->quantum < 1000) {
- pr_warn("HTB: quantum of class %X is small. Consider r2q change.\n",
- cl->common.classid);
+ warn = -1;
cl->quantum = 1000;
}
if (!hopt->quantum && cl->quantum > 200000) {
- pr_warn("HTB: quantum of class %X is big. Consider r2q change.\n",
- cl->common.classid);
+ warn = 1;
cl->quantum = 200000;
}
if (hopt->quantum)
@@ -1519,6 +1518,10 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
sch_tree_unlock(sch);
+ if (warn)
+ pr_warn("HTB: quantum of class %X is %s. Consider r2q change.\n",
+ cl->common.classid, (warn == -1 ? "small" : "big"));
+
qdisc_class_hash_grow(sch, &q->clhash);
*arg = (unsigned long)cl;
--
2.11.0
^ permalink raw reply related
* [PATCH net-next V2] net: hns3: remove unnecessary pci_set_drvdata() and devm_kfree()
From: Wei Yongjun @ 2018-03-30 2:24 UTC (permalink / raw)
To: Yisen Zhuang, Salil Mehta; +Cc: Wei Yongjun, netdev, kernel-janitors
In-Reply-To: <1522241461-77556-1-git-send-email-weiyongjun1@huawei.com>
There is no need for explicit calls of devm_kfree(), as the allocated
memory will be freed during driver's detach.
The driver core clears the driver data to NULL after device_release.
Thus, it is not needed to manually clear the device driver data to NULL.
So remove the unnecessary pci_set_drvdata() and devm_kfree().
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
v1 -> v2: change commit log
---
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index a31b4ad..8c55965 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -1614,10 +1614,6 @@ static void hns3_remove(struct pci_dev *pdev)
struct hnae3_ae_dev *ae_dev = pci_get_drvdata(pdev);
hnae3_unregister_ae_dev(ae_dev);
-
- devm_kfree(&pdev->dev, ae_dev);
-
- pci_set_drvdata(pdev, NULL);
}
static struct pci_driver hns3_driver = {
^ permalink raw reply related
* Re: [PATCH net] vhost: validate log when IOTLB is enabled
From: Jason Wang @ 2018-03-30 2:27 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20180329173438-mutt-send-email-mst@kernel.org>
On 2018年03月29日 22:44, Michael S. Tsirkin wrote:
> On Thu, Mar 29, 2018 at 04:00:04PM +0800, Jason Wang wrote:
>> Vq log_base is the userspace address of bitmap which has nothing to do
>> with IOTLB. So it needs to be validated unconditionally otherwise we
>> may try use 0 as log_base which may lead to pin pages that will lead
>> unexpected result (e.g trigger BUG_ON() in set_bit_to_user()).
>>
>> Fixes: 6b1e6cc7855b0 ("vhost: new device IOTLB API")
>> Reported-by:syzbot+6304bf97ef436580fede@syzkaller.appspotmail.com
>> Signed-off-by: Jason Wang<jasowang@redhat.com>
> One follow-up question:
>
> We still observe that get user pages returns 0 sometimes. While I agree
> we should not pass in unvalidated addresses, isn't this worth
> documenting?
>
>
Looking at get_user_pages_fast(), it has:
if (unlikely(!access_ok(write ? VERIFY_WRITE : VERIFY_READ,
(void __user *)start, len)))
return 0;
So this is expected I think.
Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC PATCH V2 8/8] vhost: event suppression for packed ring
From: Jason Wang @ 2018-03-30 2:46 UTC (permalink / raw)
To: Tiwei Bie; +Cc: mst, virtualization, kvm, netdev, linux-kernel
In-Reply-To: <20180330020518.tja3lxkfz7mdwasj@debian>
On 2018年03月30日 10:05, Tiwei Bie wrote:
> On Mon, Mar 26, 2018 at 11:38:53AM +0800, Jason Wang wrote:
>> This patch introduces basic support for event suppression aka driver
>> and device area. Compile tested only.
>>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
> [...]
>> +
>> +static bool vhost_notify_packed(struct vhost_dev *dev,
>> + struct vhost_virtqueue *vq)
>> +{
>> + __virtio16 event_off_wrap, event_flags;
>> + __u16 old, new;
>> + bool v, wrap;
>> + int off;
>> +
>> + /* Flush out used descriptors updates. This is paired
>> + * with the barrier that the Guest executes when enabling
>> + * interrupts.
>> + */
>> + smp_mb();
>> +
>> + if (vhost_get_avail(vq, event_flags,
>> + &vq->driver_event->desc_event_flags) < 0) {
>> + vq_err(vq, "Failed to get driver desc_event_flags");
>> + return true;
>> + }
>> +
>> + if (!(event_flags & cpu_to_vhost16(vq, RING_EVENT_FLAGS_DESC)))
>> + return event_flags ==
>> + cpu_to_vhost16(vq, RING_EVENT_FLAGS_ENABLE);
> Maybe it would be better to not use '&' here. Because these flags
> are not defined as bits which can be ORed or ANDed. Instead, they
> are defined as values:
>
> 0x0 enable
> 0x1 disable
> 0x2 desc
> 0x3 reserved
Yes the code seems tricky. Let me fix it in next version.
>
>> +
>> + /* Read desc event flags before event_off and event_wrap */
>> + smp_rmb();
>> +
>> + if (vhost_get_avail(vq, event_off_wrap,
>> + &vq->driver_event->desc_event_off_warp) < 0) {
>> + vq_err(vq, "Failed to get driver desc_event_off/wrap");
>> + return true;
>> + }
>> +
>> + off = vhost16_to_cpu(vq, event_off_wrap);
>> +
>> + wrap = off & 0x1;
>> + off >>= 1;
> Based on the below definitions in spec, wrap counter is
> the most significant bit.
>
> struct pvirtq_event_suppress {
> le16 {
> desc_event_off : 15; /* Descriptor Ring Change Event Offset */
> desc_event_wrap : 1; /* Descriptor Ring Change Event Wrap Counter */
> } desc; /* If desc_event_flags set to RING_EVENT_FLAGS_DESC */
> le16 {
> desc_event_flags : 2, /* Descriptor Ring Change Event Flags */
> reserved : 14; /* Reserved, set to 0 */
> } flags;
> };
Will fix this in next version.
>
>> +
>> +
>> + old = vq->signalled_used;
>> + v = vq->signalled_used_valid;
>> + new = vq->signalled_used = vq->last_used_idx;
>> + vq->signalled_used_valid = true;
>> +
>> + if (unlikely(!v))
>> + return true;
>> +
>> + return vhost_vring_packed_need_event(vq, new, old, off) &&
>> + wrap == vq->used_wrap_counter;
>> +}
>> +
>> +static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
>> +{
>> + if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED))
>> + return vhost_notify_packed(dev, vq);
>> + else
>> + return vhost_notify_split(dev, vq);
>> +}
>> +
>> /* This actually signals the guest, using eventfd. */
>> void vhost_signal(struct vhost_dev *dev, struct vhost_virtqueue *vq)
>> {
>> @@ -2789,7 +2911,17 @@ static bool vhost_enable_notify_packed(struct vhost_dev *dev,
>> __virtio16 flags;
>> int ret;
>>
>> - /* FIXME: disable notification through device area */
>> + if (!(vq->used_flags & VRING_USED_F_NO_NOTIFY))
>> + return false;
>> + vq->used_flags &= ~VRING_USED_F_NO_NOTIFY;
>> +
>> + flags = cpu_to_vhost16(vq, RING_EVENT_FLAGS_ENABLE);
>> + ret = vhost_update_device_flags(vq, flags);
>> + if (ret) {
>> + vq_err(vq, "Failed to enable notification at %p: %d\n",
>> + &vq->device_event->desc_event_flags, ret);
>> + return false;
>> + }
>>
>> /* They could have slipped one in as we were doing that: make
>> * sure it's written, then check again. */
>> @@ -2855,7 +2987,18 @@ EXPORT_SYMBOL_GPL(vhost_enable_notify);
>> static void vhost_disable_notify_packed(struct vhost_dev *dev,
>> struct vhost_virtqueue *vq)
>> {
>> - /* FIXME: disable notification through device area */
>> + __virtio16 flags;
>> + int r;
>> +
>> + if (vq->used_flags & VRING_USED_F_NO_NOTIFY)
>> + return;
>> + vq->used_flags |= VRING_USED_F_NO_NOTIFY;
>> +
>> + flags = cpu_to_vhost16(vq, RING_EVENT_FLAGS_DISABLE);
>> + r = vhost_update_device_flags(vq, flags);
>> + if (r)
>> + vq_err(vq, "Failed to enable notification at %p: %d\n",
>> + &vq->device_event->desc_event_flags, r);
>> }
>>
>> static void vhost_disable_notify_split(struct vhost_dev *dev,
>> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
>> index 8a9df4f..02d7a36 100644
>> --- a/drivers/vhost/vhost.h
>> +++ b/drivers/vhost/vhost.h
>> @@ -96,8 +96,14 @@ struct vhost_virtqueue {
>> struct vring_desc __user *desc;
>> struct vring_desc_packed __user *desc_packed;
> Do you think it'd be better to name the desc type as
> struct vring_packed_desc?
Ok. Will do.
> And it will be consistent
> with other names, like:
>
> struct vring_packed;
> struct vring_packed_desc_event;
>
>> };
>> - struct vring_avail __user *avail;
>> - struct vring_used __user *used;
>> + union {
>> + struct vring_avail __user *avail;
>> + struct vring_packed_desc_event __user *driver_event;
>> + };
>> + union {
>> + struct vring_used __user *used;
>> + struct vring_packed_desc_event __user *device_event;
>> + };
>> const struct vhost_umem_node *meta_iotlb[VHOST_NUM_ADDRS];
>> struct file *kick;
>> struct eventfd_ctx *call_ctx;
>> diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
>> index e297580..7cdbf06 100644
>> --- a/include/uapi/linux/virtio_ring.h
>> +++ b/include/uapi/linux/virtio_ring.h
>> @@ -75,6 +75,25 @@ struct vring_desc_packed {
>> __virtio16 flags;
>> };
>>
>> +/* Enable events */
>> +#define RING_EVENT_FLAGS_ENABLE 0x0
>> +/* Disable events */
>> +#define RING_EVENT_FLAGS_DISABLE 0x1
>> +/*
>> + * Enable events for a specific descriptor
>> + * (as specified by Descriptor Ring Change Event Offset/Wrap Counter).
>> + * Only valid if VIRTIO_F_RING_EVENT_IDX has been negotiated.
>> + */
>> +#define RING_EVENT_FLAGS_DESC 0x2
>> +/* The value 0x3 is reserved */
>> +
>> +struct vring_packed_desc_event {
>> + /* Descriptor Ring Change Event Offset and Wrap Counter */
>> + __virtio16 desc_event_off_warp;
>> + /* Descriptor Ring Change Event Flags */
>> + __virtio16 desc_event_flags;
> Do you think it'd be better to remove the prefix (desc_event_) for
> the fields. And it will be consistent with other definitions, e.g.:
>
> struct vring_packed_desc {
> /* Buffer Address. */
> __virtio64 addr;
> /* Buffer Length. */
> __virtio32 len;
> /* Buffer ID. */
> __virtio16 id;
> /* The flags depending on descriptor type. */
> __virtio16 flags;
> };
Yes. Let me do it in next version.
Thanks for the review!
>> +};
>> +
>> /* Virtio ring descriptors: 16 bytes. These can chain together via "next". */
>> struct vring_desc {
>> /* Address (guest-physical). */
>> --
>> 2.7.4
>>
^ permalink raw reply
* Re: [PATCH net-next] net/mlx4_en: CHECKSUM_COMPLETE support for fragments
From: Eric Dumazet @ 2018-03-30 3:32 UTC (permalink / raw)
To: Saeed Mahameed
Cc: David Miller, netdev, Eric Dumazet, Willem de Bruijn,
Tariq Toukan
In-Reply-To: <CALzJLG8YCOm8bCcTBDbGbEzZdJZ6QRTkfoTNYRvEjA9QQyBZFA@mail.gmail.com>
On Thu, Mar 29, 2018 at 5:44 PM Saeed Mahameed <saeedm@dev.mellanox.co.il>
wrote:
> On Thu, Mar 29, 2018 at 11:07 AM, David Miller <davem@davemloft.net>
wrote:
> > From: Eric Dumazet <edumazet@google.com>
> > Date: Tue, 27 Mar 2018 14:21:14 -0700
> >
> >> Refine the RX check summing handling to propagate the
> >> hardware provided checksum so that we do not have to
> >> compute it later in software.
> >>
> >> Signed-off-by: Eric Dumazet <edumazet@google.com>
> >
> > Tariq, please review.
> Hi Dave, Eric.
> The patch looks ok but i would let tariq review it and decide if he
> wants to run full regression coverage on it
> since it changes the default behavior of the driver's checksum reporting.
> It is already weekend for him and for the team in Israel, and i don't
> think this can be handled before next week :).
> So it is really up to you guys.
> Thanks,
> Saeed.
Hi Saaed
This definitely can wait, nothing urgent really.
Thanks.
^ 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