* Re: [PATCH net-next v6 19/23] zinc: Curve25519 ARM implementation
From: Jason A. Donenfeld @ 2018-10-03 3:10 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: LKML, Netdev, Linux Crypto Mailing List, David Miller,
Greg Kroah-Hartman, Samuel Neves, Andrew Lutomirski,
Jean-Philippe Aumasson, Russell King - ARM Linux,
linux-arm-kernel
In-Reply-To: <CAKv+Gu9FLDRLxHReKcveZYHNYerR5Y2pZd9gn-hWrU0jb2KgfA@mail.gmail.com>
On Tue, Oct 2, 2018 at 6:59 PM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> Could you elaborate on what qhasm is exactly? And, as with the other
> patches, I would prefer it if we could have your changes as a separate
> patch (although having the qhasm base would be preferred)
By the way, as of a few minutes ago, if you look in the development
tree at the commit called "zinc: Curve25519 ARM implementation", that
now shows the diffs to the original, as you requested. I'll probably
obsess over that a little bit more before v7, but if you see anything
gnarly there beforehand, I'd be happy to hear.
Jason
^ permalink raw reply
* Re: [PATCH net-next v2 00/10] vrf: allow simultaneous service instances in default and other VRFs
From: David Ahern @ 2018-10-02 20:16 UTC (permalink / raw)
To: Mike Manning, netdev
In-Reply-To: <20181001084320.32453-1-mmanning@vyatta.att-mail.com>
On 10/1/18 2:43 AM, Mike Manning wrote:
> Services currently have to be VRF-aware if they are using an unbound
> socket. One cannot have multiple service instances running in the
> default and other VRFs for services that are not VRF-aware and listen
> on an unbound socket. This is because there is no way of isolating
> packets received in the default VRF from those arriving in other VRFs.
>
> This series provides this isolation subject to the existing kernel
> parameter net.ipv4.tcp_l3mdev_accept not being set, given that this is
> documented as allowing a single service instance to work across all
> VRF domains. The functionality applies to UDP & TCP services, for IPv4
> and IPv6, in particular adding VRF table handling for IPv6 multicast.
>
I see 1 failure caused by this patch set: IPv6/UDP send to a peer's
linklocal address with no server on port in the peer. An ICMP
unreachable is expected since there is no server and it is not received.
Happens with or without net.ipv4.udp_l3mdev_accept set.
^ permalink raw reply
* __dev_kfree_skb_any() and use of dev_kfree_skb()
From: Florian Fainelli @ 2018-10-02 20:07 UTC (permalink / raw)
To: netdev, edumazet, nhorman; +Cc: davem
Hi Eric, Neil,
Should not __dev_kfree_skb_any() call kfree_skb() instead of
dev_kfree_skb() which is aliased to consumes_skb() and therefore does
not flag the skb with SKB_REASON_DROPPED?
If we take the in_irq() || irqs_disabled() branch, we will be calling
__dev_kfree_skb_irq() which takes care of setting the skb_free_reason
frmo the caller.
Is there an implied semantic with dev_kfree_skb() that it means it was
freed by the network device and therefore this equals to a consumption
(not a drop)? The comment above dev_kfree_skb_any() seems to imply this
should be a context unaware replacement for kfree_skb().
Thanks!
--
Florian
^ permalink raw reply
* Re: [PATCH 1/3] bpf: allow zero-initializing hash map seed
From: Jann Horn @ 2018-10-02 19:59 UTC (permalink / raw)
To: lmb; +Cc: Alexei Starovoitov, Daniel Borkmann, Network Development,
Linux API
In-Reply-To: <20181001104509.24211-2-lmb@cloudflare.com>
On Mon, Oct 1, 2018 at 12:47 PM Lorenz Bauer <lmb@cloudflare.com> wrote:
>
> Add a new flag BPF_F_ZERO_SEED, which forces a hash map
> to initialize the seed to zero.
> ---
> include/uapi/linux/bpf.h | 2 ++
> kernel/bpf/hashtab.c | 8 ++++++--
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index aa5ccd2385ed..9d15c8f179ac 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -252,6 +252,8 @@ enum bpf_attach_type {
> #define BPF_F_NO_COMMON_LRU (1U << 1)
> /* Specify numa node during map creation */
> #define BPF_F_NUMA_NODE (1U << 2)
> +/* Zero-initialize hash function seed */
> +#define BPF_F_ZERO_SEED (1U << 6)
>
> /* flags for BPF_PROG_QUERY */
> #define BPF_F_QUERY_EFFECTIVE (1U << 0)
> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 2c1790288138..a79e123dae62 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -23,7 +23,7 @@
>
> #define HTAB_CREATE_FLAG_MASK \
> (BPF_F_NO_PREALLOC | BPF_F_NO_COMMON_LRU | BPF_F_NUMA_NODE | \
> - BPF_F_RDONLY | BPF_F_WRONLY)
> + BPF_F_RDONLY | BPF_F_WRONLY | BPF_F_ZERO_SEED)
>
> struct bucket {
> struct hlist_nulls_head head;
> @@ -373,7 +373,11 @@ static struct bpf_map *htab_map_alloc(union bpf_attr *attr)
> if (!htab->buckets)
> goto free_htab;
>
> - htab->hashrnd = get_random_int();
> + if (htab->map.map_flags & BPF_F_ZERO_SEED)
> + htab->hashrnd = 0;
> + else
> + htab->hashrnd = get_random_int();
> +
If this is for testing only, you can slap a capable(CAP_SYS_ADMIN)
check in here, right? I doubt it matters, but I don't really like
seeing something like this exposed to unprivileged userspace just
because you need it for kernel testing.
^ permalink raw reply
* Re: [PATCH net-next v2 06/10] net: IP[V6]_MULTICAST_IF constraint on unbound socket if VRFs present
From: David Ahern @ 2018-10-02 19:59 UTC (permalink / raw)
To: Mike Manning, netdev; +Cc: Duncan Eastoe
In-Reply-To: <20181001084320.32453-7-mmanning@vyatta.att-mail.com>
On 10/1/18 2:43 AM, Mike Manning wrote:
> From: Duncan Eastoe <deastoe@vyatta.att-mail.com>
>
> If setsockopt(IP_MULTICAST_IF) or setsockopt(IPV6_MULTICAST_IF) is
> called on a socket which is not bound to a VRF then we should ensure
> that the output device chosen is also not bound to a VRF master.
Why does it matter? An app can set IP_MULTICAST_IF on the socket in the
same way it can set the device bind.
This breaks existing apps.
>
> This avoids inadvertently sending traffic out of the wrong interface.
> This can be particularly problematic for IP_MULTICAST_IF since the
> interface lookup can be performed by address as well as ifindex. If
> there are interfaces with the same address, one unbound and one bound
> to a VRF, then the interface bound to the VRF may be chosen when the
> sockopt is called on an unbound socket.
>
> Signed-off-by: Duncan Eastoe <deastoe@vyatta.att-mail.com>
> Signed-off-by: Mike Manning <mmanning@vyatta.att-mail.com>
> ---
> net/ipv4/ip_sockglue.c | 3 +++
> net/ipv6/ipv6_sockglue.c | 3 +++
> 2 files changed, 6 insertions(+)
>
> diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
> index c0fe5ad996f2..026971314c43 100644
> --- a/net/ipv4/ip_sockglue.c
> +++ b/net/ipv4/ip_sockglue.c
> @@ -892,6 +892,9 @@ static int do_ip_setsockopt(struct sock *sk, int level,
> dev_put(dev);
>
> err = -EINVAL;
> + if (!sk->sk_bound_dev_if && midx)
> + break;
> +
> if (sk->sk_bound_dev_if &&
> mreq.imr_ifindex != sk->sk_bound_dev_if &&
> (!midx || midx != sk->sk_bound_dev_if))
> diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
> index c0cac9cc3a28..7dfbc797b130 100644
> --- a/net/ipv6/ipv6_sockglue.c
> +++ b/net/ipv6/ipv6_sockglue.c
> @@ -626,6 +626,9 @@ static int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
>
> rcu_read_unlock();
>
> + if (!sk->sk_bound_dev_if && midx)
> + goto e_inval;
> +
> if (sk->sk_bound_dev_if &&
> sk->sk_bound_dev_if != val &&
> (!midx || midx != sk->sk_bound_dev_if))
>
^ permalink raw reply
* [Patch net-next] net_sched: convert idrinfo->lock from spinlock to a mutex
From: Cong Wang @ 2018-10-02 19:50 UTC (permalink / raw)
To: netdev; +Cc: jiri, jhs, vladbu, idosch, Cong Wang, Jiri Pirko
In commit ec3ed293e766 ("net_sched: change tcf_del_walker() to take idrinfo->lock")
we move fl_hw_destroy_tmplt() to a workqueue to avoid blocking
with the spinlock held. Unfortunately, this causes a lot of
troubles here:
1. tcf_chain_destroy() could be called right after we queue the work
but before the work runs. This is a use-after-free.
2. The chain refcnt is already 0, we can't even just hold it again.
We can check refcnt==1 but it is ugly.
3. The chain with refcnt 0 is still visible in its block, which means
it could be still found and used!
4. The block has a refcnt too, we can't hold it without introducing a
proper API either.
We can make it working but the end result is ugly. Instead of wasting
time on reviewing it, let's just convert the troubling spinlock to
a mutex, which allows us to use non-atomic allocations too.
Fixes: ec3ed293e766 ("net_sched: change tcf_del_walker() to take idrinfo->lock")
Reported-by: Ido Schimmel <idosch@idosch.org>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Vlad Buslov <vladbu@mellanox.com>
Cc: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
include/net/act_api.h | 4 ++--
net/sched/act_api.c | 44 ++++++++++++++++++++++----------------------
net/sched/cls_flower.c | 13 ++-----------
3 files changed, 26 insertions(+), 35 deletions(-)
diff --git a/include/net/act_api.h b/include/net/act_api.h
index 1ddff3360592..05c7df41d737 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -13,7 +13,7 @@
#include <net/netns/generic.h>
struct tcf_idrinfo {
- spinlock_t lock;
+ struct mutex lock;
struct idr action_idr;
};
@@ -117,7 +117,7 @@ int tc_action_net_init(struct tc_action_net *tn,
if (!tn->idrinfo)
return -ENOMEM;
tn->ops = ops;
- spin_lock_init(&tn->idrinfo->lock);
+ mutex_init(&tn->idrinfo->lock);
idr_init(&tn->idrinfo->action_idr);
return err;
}
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 3c7c23421885..55153da00278 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -104,11 +104,11 @@ static int __tcf_action_put(struct tc_action *p, bool bind)
{
struct tcf_idrinfo *idrinfo = p->idrinfo;
- if (refcount_dec_and_lock(&p->tcfa_refcnt, &idrinfo->lock)) {
+ if (refcount_dec_and_mutex_lock(&p->tcfa_refcnt, &idrinfo->lock)) {
if (bind)
atomic_dec(&p->tcfa_bindcnt);
idr_remove(&idrinfo->action_idr, p->tcfa_index);
- spin_unlock(&idrinfo->lock);
+ mutex_unlock(&idrinfo->lock);
tcf_action_cleanup(p);
return 1;
@@ -200,7 +200,7 @@ static int tcf_dump_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
struct tc_action *p;
unsigned long id = 1;
- spin_lock(&idrinfo->lock);
+ mutex_lock(&idrinfo->lock);
s_i = cb->args[0];
@@ -235,7 +235,7 @@ static int tcf_dump_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
if (index >= 0)
cb->args[0] = index + 1;
- spin_unlock(&idrinfo->lock);
+ mutex_unlock(&idrinfo->lock);
if (n_i) {
if (act_flags & TCA_FLAG_LARGE_DUMP_ON)
cb->args[1] = n_i;
@@ -277,18 +277,18 @@ static int tcf_del_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
if (nla_put_string(skb, TCA_KIND, ops->kind))
goto nla_put_failure;
- spin_lock(&idrinfo->lock);
+ mutex_lock(&idrinfo->lock);
idr_for_each_entry_ul(idr, p, id) {
ret = tcf_idr_release_unsafe(p);
if (ret == ACT_P_DELETED) {
module_put(ops->owner);
n_i++;
} else if (ret < 0) {
- spin_unlock(&idrinfo->lock);
+ mutex_unlock(&idrinfo->lock);
goto nla_put_failure;
}
}
- spin_unlock(&idrinfo->lock);
+ mutex_unlock(&idrinfo->lock);
if (nla_put_u32(skb, TCA_FCNT, n_i))
goto nla_put_failure;
@@ -324,13 +324,13 @@ int tcf_idr_search(struct tc_action_net *tn, struct tc_action **a, u32 index)
struct tcf_idrinfo *idrinfo = tn->idrinfo;
struct tc_action *p;
- spin_lock(&idrinfo->lock);
+ mutex_lock(&idrinfo->lock);
p = idr_find(&idrinfo->action_idr, index);
if (IS_ERR(p))
p = NULL;
else if (p)
refcount_inc(&p->tcfa_refcnt);
- spin_unlock(&idrinfo->lock);
+ mutex_unlock(&idrinfo->lock);
if (p) {
*a = p;
@@ -345,10 +345,10 @@ static int tcf_idr_delete_index(struct tcf_idrinfo *idrinfo, u32 index)
struct tc_action *p;
int ret = 0;
- spin_lock(&idrinfo->lock);
+ mutex_lock(&idrinfo->lock);
p = idr_find(&idrinfo->action_idr, index);
if (!p) {
- spin_unlock(&idrinfo->lock);
+ mutex_unlock(&idrinfo->lock);
return -ENOENT;
}
@@ -358,7 +358,7 @@ static int tcf_idr_delete_index(struct tcf_idrinfo *idrinfo, u32 index)
WARN_ON(p != idr_remove(&idrinfo->action_idr,
p->tcfa_index));
- spin_unlock(&idrinfo->lock);
+ mutex_unlock(&idrinfo->lock);
tcf_action_cleanup(p);
module_put(owner);
@@ -369,7 +369,7 @@ static int tcf_idr_delete_index(struct tcf_idrinfo *idrinfo, u32 index)
ret = -EPERM;
}
- spin_unlock(&idrinfo->lock);
+ mutex_unlock(&idrinfo->lock);
return ret;
}
@@ -431,10 +431,10 @@ void tcf_idr_insert(struct tc_action_net *tn, struct tc_action *a)
{
struct tcf_idrinfo *idrinfo = tn->idrinfo;
- spin_lock(&idrinfo->lock);
+ mutex_lock(&idrinfo->lock);
/* Replace ERR_PTR(-EBUSY) allocated by tcf_idr_check_alloc */
WARN_ON(!IS_ERR(idr_replace(&idrinfo->action_idr, a, a->tcfa_index)));
- spin_unlock(&idrinfo->lock);
+ mutex_unlock(&idrinfo->lock);
}
EXPORT_SYMBOL(tcf_idr_insert);
@@ -444,10 +444,10 @@ void tcf_idr_cleanup(struct tc_action_net *tn, u32 index)
{
struct tcf_idrinfo *idrinfo = tn->idrinfo;
- spin_lock(&idrinfo->lock);
+ mutex_lock(&idrinfo->lock);
/* Remove ERR_PTR(-EBUSY) allocated by tcf_idr_check_alloc */
WARN_ON(!IS_ERR(idr_remove(&idrinfo->action_idr, index)));
- spin_unlock(&idrinfo->lock);
+ mutex_unlock(&idrinfo->lock);
}
EXPORT_SYMBOL(tcf_idr_cleanup);
@@ -465,14 +465,14 @@ int tcf_idr_check_alloc(struct tc_action_net *tn, u32 *index,
int ret;
again:
- spin_lock(&idrinfo->lock);
+ mutex_lock(&idrinfo->lock);
if (*index) {
p = idr_find(&idrinfo->action_idr, *index);
if (IS_ERR(p)) {
/* This means that another process allocated
* index but did not assign the pointer yet.
*/
- spin_unlock(&idrinfo->lock);
+ mutex_unlock(&idrinfo->lock);
goto again;
}
@@ -485,7 +485,7 @@ int tcf_idr_check_alloc(struct tc_action_net *tn, u32 *index,
} else {
*a = NULL;
ret = idr_alloc_u32(&idrinfo->action_idr, NULL, index,
- *index, GFP_ATOMIC);
+ *index, GFP_KERNEL);
if (!ret)
idr_replace(&idrinfo->action_idr,
ERR_PTR(-EBUSY), *index);
@@ -494,12 +494,12 @@ int tcf_idr_check_alloc(struct tc_action_net *tn, u32 *index,
*index = 1;
*a = NULL;
ret = idr_alloc_u32(&idrinfo->action_idr, NULL, index,
- UINT_MAX, GFP_ATOMIC);
+ UINT_MAX, GFP_KERNEL);
if (!ret)
idr_replace(&idrinfo->action_idr, ERR_PTR(-EBUSY),
*index);
}
- spin_unlock(&idrinfo->lock);
+ mutex_unlock(&idrinfo->lock);
return ret;
}
EXPORT_SYMBOL(tcf_idr_check_alloc);
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 92dd5071a708..9aada2d0ef06 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -79,7 +79,6 @@ struct fl_flow_tmplt {
struct fl_flow_key mask;
struct flow_dissector dissector;
struct tcf_chain *chain;
- struct rcu_work rwork;
};
struct cls_fl_head {
@@ -1438,20 +1437,12 @@ static void *fl_tmplt_create(struct net *net, struct tcf_chain *chain,
return ERR_PTR(err);
}
-static void fl_tmplt_destroy_work(struct work_struct *work)
-{
- struct fl_flow_tmplt *tmplt = container_of(to_rcu_work(work),
- struct fl_flow_tmplt, rwork);
-
- fl_hw_destroy_tmplt(tmplt->chain, tmplt);
- kfree(tmplt);
-}
-
static void fl_tmplt_destroy(void *tmplt_priv)
{
struct fl_flow_tmplt *tmplt = tmplt_priv;
- tcf_queue_work(&tmplt->rwork, fl_tmplt_destroy_work);
+ fl_hw_destroy_tmplt(tmplt->chain, tmplt);
+ kfree(tmplt);
}
static int fl_dump_key_val(struct sk_buff *skb,
--
2.14.4
^ permalink raw reply related
* Re: [net-next 0/9][pull request] 100GbE Intel Wired LAN Driver Updates 2018-10-02
From: Jeff Kirsher @ 2018-10-02 19:47 UTC (permalink / raw)
To: David Miller; +Cc: netdev, nhorman, sassmann, jogreene
In-Reply-To: <20181002.114109.2250028418917161969.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 395 bytes --]
On Tue, 2018-10-02 at 11:41 -0700, David Miller wrote:
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Tue, 2 Oct 2018 08:24:38 -0700
>
> > This series contains updates to ice driver only.
>
> If there was a problem yo I'll solve it.
> Check out the hook while my DJ revolves it.
>
> Ice ice baby.
> Ice ice baby.
Thanks, now I can't get that song out of my head...
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH net-next v2 04/10] net: provide a sysctl raw_l3mdev_accept for raw socket lookup with VRFs
From: David Ahern @ 2018-10-02 19:39 UTC (permalink / raw)
To: Mike Manning, netdev
In-Reply-To: <20181001084320.32453-5-mmanning@vyatta.att-mail.com>
On 10/1/18 2:43 AM, Mike Manning wrote:
> Add a sysctl raw_l3mdev_accept to control raw socket lookup in a manner
> similar to use of tcp_l3mdev_accept for stream and of udp_l3mdev_accept
> for datagram sockets. Have this default to off as this is what users
> expect, given that there is no explicit mechanism to set unmodified
> VRF-unaware application into a default VRF.
It needs to default to on to avoid breaking existing users. With the
raw_l3mdev_accept sysctl set to 0 both 'ping -I <vrf> <addr>' and 'ping
-I <dev> <addr>' fail when they succeed today.
^ permalink raw reply
* Re: [PATCH net-next v2 01/10] net: allow binding socket in a VRF when there's an unbound socket
From: David Ahern @ 2018-10-02 19:39 UTC (permalink / raw)
To: Mike Manning, netdev; +Cc: Robert Shearman
In-Reply-To: <20181001084320.32453-2-mmanning@vyatta.att-mail.com>
On 10/1/18 2:43 AM, Mike Manning wrote:
> There is no easy way currently for applications that want to receive
> packets in the default VRF to be isolated from packets arriving in
> VRFs, which makes using VRF-unaware applications in a VRF-aware system
> a potential security risk.
please drop that paragraph from the commit message. It is misleading and
wrong.
Without VRF I can start ssh bound to wildcard address and port 22
allowing connections across any interface in the box. If I do not want
that sestup, I have options: e.g., bind ssh to the management address or
install netfilter rules. The same applies with VRF as I mentioned in the
v1 review.
You not liking the options or wanting another option is a different
reason for the change than claiming the current options are a security risk.
^ permalink raw reply
* [PATCH net] inet: make sure to grab rcu_read_lock before using ireq->ireq_opt
From: Eric Dumazet @ 2018-10-02 19:35 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
Timer handlers do not imply rcu_read_lock(), so my recent fix
triggered a LOCKDEP warning when SYNACK is retransmit.
Lets add rcu_read_lock()/rcu_read_unlock() pairs around ireq->ireq_opt
usages instead of guessing what is done by callers, since it is
not worth the pain.
Get rid of ireq_opt_deref() helper since it hides the logic
without real benefit, since it is now a standard rcu_dereference().
Fixes: 1ad98e9d1bdf ("tcp/dccp: fix lockdep issue when SYN is backlogged")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Willem de Bruijn <willemb@google.com>
---
include/net/inet_sock.h | 5 -----
net/dccp/ipv4.c | 4 +++-
net/ipv4/inet_connection_sock.c | 5 ++++-
net/ipv4/tcp_ipv4.c | 4 +++-
4 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index a8cd5cf9ff5b6ddc50bd2e70d3f9103afa32a3b5..a80fd0ac4563283246f4f53cea1ac0cd17b41dab 100644
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -130,11 +130,6 @@ static inline int inet_request_bound_dev_if(const struct sock *sk,
return sk->sk_bound_dev_if;
}
-static inline struct ip_options_rcu *ireq_opt_deref(const struct inet_request_sock *ireq)
-{
- return rcu_dereference(ireq->ireq_opt);
-}
-
struct inet_cork {
unsigned int flags;
__be32 addr;
diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
index b08feb219b44b67eadf408a33649d8c7ec9db2d0..8e08cea6f17866b5fb1619f570de747c6a837cbd 100644
--- a/net/dccp/ipv4.c
+++ b/net/dccp/ipv4.c
@@ -493,9 +493,11 @@ static int dccp_v4_send_response(const struct sock *sk, struct request_sock *req
dh->dccph_checksum = dccp_v4_csum_finish(skb, ireq->ir_loc_addr,
ireq->ir_rmt_addr);
+ rcu_read_lock();
err = ip_build_and_send_pkt(skb, sk, ireq->ir_loc_addr,
ireq->ir_rmt_addr,
- ireq_opt_deref(ireq));
+ rcu_dereference(ireq->ireq_opt));
+ rcu_read_unlock();
err = net_xmit_eval(err);
}
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index dfd5009f96ef7111a593651a48f73c4a92c3ed15..15e7f7915a21e0fbce09d5d2c17d877eae499e03 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -544,7 +544,8 @@ struct dst_entry *inet_csk_route_req(const struct sock *sk,
struct ip_options_rcu *opt;
struct rtable *rt;
- opt = ireq_opt_deref(ireq);
+ rcu_read_lock();
+ opt = rcu_dereference(ireq->ireq_opt);
flowi4_init_output(fl4, ireq->ir_iif, ireq->ir_mark,
RT_CONN_FLAGS(sk), RT_SCOPE_UNIVERSE,
@@ -558,11 +559,13 @@ struct dst_entry *inet_csk_route_req(const struct sock *sk,
goto no_route;
if (opt && opt->opt.is_strictroute && rt->rt_uses_gateway)
goto route_err;
+ rcu_read_unlock();
return &rt->dst;
route_err:
ip_rt_put(rt);
no_route:
+ rcu_read_unlock();
__IP_INC_STATS(net, IPSTATS_MIB_OUTNOROUTES);
return NULL;
}
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 44c09eddbb781c03da2417aaa925e360de01a6e9..cd426313a29819b34648086b551fe9390d8a0b0a 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -943,9 +943,11 @@ static int tcp_v4_send_synack(const struct sock *sk, struct dst_entry *dst,
if (skb) {
__tcp_v4_send_check(skb, ireq->ir_loc_addr, ireq->ir_rmt_addr);
+ rcu_read_lock();
err = ip_build_and_send_pkt(skb, sk, ireq->ir_loc_addr,
ireq->ir_rmt_addr,
- ireq_opt_deref(ireq));
+ rcu_dereference(ireq->ireq_opt));
+ rcu_read_unlock();
err = net_xmit_eval(err);
}
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related
* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2018-10-03 2:18 UTC (permalink / raw)
To: David Miller, Networking
Cc: Linux-Next Mailing List, Linux Kernel Mailing List, David Ahern,
Christian Brauner
[-- Attachment #1: Type: text/plain, Size: 1560 bytes --]
Hi all,
Today's linux-next merge of the net-next tree got a conflict in:
net/core/rtnetlink.c
between commit:
893626d6a353 ("rtnetlink: Fail dump if target netnsid is invalid")
from the net tree and commits:
c383edc42403 ("rtnetlink: add rtnl_get_net_ns_capable()")
7e4a8d5a93f6 ("rtnetlink: s/IFLA_IF_NETNSID/IFLA_TARGET_NETNSID/g")
from the net-next tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc net/core/rtnetlink.c
index 7f37fe9c65a5,35162e1b06ad..000000000000
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@@ -1895,11 -1910,13 +1910,11 @@@ static int rtnl_dump_ifinfo(struct sk_b
if (nlmsg_parse(cb->nlh, hdrlen, tb, IFLA_MAX,
ifla_policy, NULL) >= 0) {
- if (tb[IFLA_IF_NETNSID]) {
- netnsid = nla_get_s32(tb[IFLA_IF_NETNSID]);
- tgt_net = get_target_net(skb->sk, netnsid);
+ if (tb[IFLA_TARGET_NETNSID]) {
+ netnsid = nla_get_s32(tb[IFLA_TARGET_NETNSID]);
+ tgt_net = rtnl_get_net_ns_capable(skb->sk, netnsid);
- if (IS_ERR(tgt_net)) {
- tgt_net = net;
- netnsid = -1;
- }
+ if (IS_ERR(tgt_net))
+ return PTR_ERR(tgt_net);
}
if (tb[IFLA_EXT_MASK])
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 1/1] bridge: remove BR_GROUPFWD_RESTRICTED for arbitrary forwarding of reserved addresses
From: Richard Weinberger @ 2018-10-02 19:30 UTC (permalink / raw)
To: nikolay
Cc: idosch, Stephen Hemminger, Florian Fainelli, Bernhard Thaler,
David S. Miller, bridge, netdev, David Gstir, roopa
In-Reply-To: <102873a5-ed43-2bf7-9ef7-70a76c5b7f6e@cumulusnetworks.com>
On Tue, Oct 2, 2018 at 6:48 PM Nikolay Aleksandrov
<nikolay@cumulusnetworks.com> wrote:
> > Is there a reason why this knob is not documented in Documentation/ABI/testing/sysfs-class-net?
> >
>
> Hmm, I don't think any of the bridge knobs are documented there or any other master device for that
> matter. Most of these options are documented in ip-link.8 man page from iproute2, it seems though
> this one I've missed to document. I'll prepare a patch for iproute2.
Well, /sys/class/net/<bridge iface>/bridge/group_fwd_mask is
documented, this confused me a little.
--
Thanks,
//richard
^ permalink raw reply
* Re: [Patch net-next v3] net_sched: change tcf_del_walker() to take idrinfo->lock
From: Cong Wang @ 2018-10-02 19:23 UTC (permalink / raw)
To: Ido Schimmel
Cc: Linux Kernel Network Developers, Jiri Pirko, Jamal Hadi Salim,
Vlad Buslov
In-Reply-To: <CAM_iQpVpmJ76vs4j0eaDWCJiXDjZA8Uj_ozS8gp5RZBpAV-zCQ@mail.gmail.com>
On Fri, Sep 28, 2018 at 9:47 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> Just FYI, the crash is fixed, but another RCU warning pops up after
> my fix. So I am still debugging it, it is related to the kfree_rcu() in
> tcf_chain_destroy().
Update: I tried very hard to make it working, the end result is ugly,
as I have to play with refcnt==1. I don't like it either. So, I am thinking
about to just convert the idrinfo->lock from a spinlock to a mutex,
which could save us a lot of troubles here.
^ permalink raw reply
* Re: [PATCH RFC v2 net-next 03/25] netlink: introduce NLM_F_DUMP_PROPER_HDR flag
From: David Ahern @ 2018-10-02 19:20 UTC (permalink / raw)
To: Jiri Benc; +Cc: Christian Brauner, netdev, davem, stephen
In-Reply-To: <20181002183018.65c41753@redhat.com>
On 10/2/18 10:30 AM, Jiri Benc wrote:
> On Tue, 2 Oct 2018 08:57:24 -0600, David Ahern wrote:
>> You can when you introduce a new option or a new flag that is required
>> to get new behavior like kernel side filtering.
>
> Yes. That was what I tried with the patchset a few years back. It would
> be nice to revive the effort.
>
>> I chose a netlink flag for consistency with NLM_F_DUMP_INTR and
>> NLM_F_DUMP_FILTERED. Both are netlink flags. This patch set fixes only
>> what is broken -- dumps.
>
> When we're introducing better input checking in netlink (which is a
> good thing!), it would be good to do it consistently and have it
> generic across all operations.
>
Thinking about this some more... a setsockopt to enable the new checks
would provide a definitive way for userspace to know if the feature is
supported or not.
As for expanding the scope to NEW (and maybe DEL) commands (which was
the point of your patch set 3 years ago), I think is an idealistic goal
that is not practical to implement. Trying to go through all commands
and cover all options and valid combinations to report back errors is a
herculean effort with little return on the time investment. New commands
and new features should certainly add rigid checks for valid
combinations, but a retrospective audit on existing commands is a time
sink. Perhaps new attributes can be checked (new command on old kernel);
seems like that can be covered by a simple change to nla_parse to handle
type > maxtype based on a new input flag.
Once the flag goes in, all of the commands to be affected by it have to
be done in the same release. I have done that for the dump commands
which is fairly easy considering the existing code.
^ permalink raw reply
* [PATCH net] bonding: fix warning message
From: Mahesh Bandewar @ 2018-10-02 19:14 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, Veaceslav Falico, David Miller
Cc: Mahesh Bandewar, Netdev, Eric Dumazet, Mahesh Bandewar
From: Mahesh Bandewar <maheshb@google.com>
RX queue config for bonding master could be different from its slave
device(s). With the commit 6a9e461f6fe4 ("bonding: pass link-local
packets to bonding master also."), the packet is reinjected into stack
with skb->dev as bonding master. This potentially triggers the
message:
"bondX received packet on queue Y, but number of RX queues is Z"
whenever the queue that packet is received on is higher than the
numrxqueues on bonding master (Y > Z).
Fixes: 6a9e461f6fe4 ("bonding: pass link-local packets to bonding master also.")
Reported-by: John Sperbeck <jsperbeck@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
drivers/net/bonding/bond_main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index c05c01a00755..ee28ec9e0aba 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1187,6 +1187,7 @@ static rx_handler_result_t bond_handle_frame(struct sk_buff **pskb)
if (nskb) {
nskb->dev = bond->dev;
+ nskb->queue_mapping = 0;
netif_rx(nskb);
}
return RX_HANDLER_PASS;
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related
* Re: [PATCH v2] net/ncsi: Add NCSI OEM command support
From: Vijay Khemka @ 2018-10-02 18:56 UTC (permalink / raw)
To: Justin.Lee1@Dell.com, sam@mendozajonas.com, joel@jms.id.au,
linux-aspeed@lists.ozlabs.org, openbmc@lists.ozlabs.org,
Sai Dasari, netdev@vger.kernel.org, christian@cmd.nu
In-Reply-To: <4876838166d140bab331280efcfd771d@AUSX13MPS306.AMER.DELL.COM>
Hi Justin,
Thanks for response. Please see my comments below.
-Vijay
> > diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
> > index 7567ca63aae2..2f98533eba46 100644
> > --- a/net/ncsi/ncsi-cmd.c
> > +++ b/net/ncsi/ncsi-cmd.c
> > @@ -211,6 +211,26 @@ static int ncsi_cmd_handler_snfc(struct sk_buff *skb,
> > return 0;
> > }
> >
> > +static int ncsi_cmd_handler_oem(struct sk_buff *skb,
> > + struct ncsi_cmd_arg *nca)
> > +{
> > + struct ncsi_cmd_oem_pkt *cmd;
>OEM command doesn't not have the fixed data size. Should we use pointer instead?
>struct ncsi_cmd_oem_pkt {
> struct ncsi_cmd_pkt_hdr cmd; /* Command header */
> __be32 mfr_id; /* Manufacture ID */
> unsigned char *data; /* OEM Payload Data */
>};
Yes, I agree that OEM command doesn't have fixed data but to map to skbuff structure,
I have defined above structure as per NCSI spec, We can certainly have a MAX_DATA_LEN define.
> > + unsigned int len;
> > +
> > + len = sizeof(struct ncsi_cmd_pkt_hdr) + 4;
> > + if (nca->payload < 26)
> > + len += 26;
>Why does it add 26? I knew the other place in ncsi_alloc_command() is also add 26.
>If it is less than 26, then it should be a fixed size of structure ncsi_cmd_pkt (46), right?
It adds 26 because It has already assigned len to hdr+4 which is 16+4 = 20 bytes. By
adding 26 it makes it to 46. I am just being consistent with other portion of code.
> > + else
> > + len += nca->payload;
> > +
> > + cmd = skb_put_zero(skb, len);
> > + cmd->mfr_id = nca->dwords[0];
> > + memcpy(cmd->data, &nca->dwords[1], nca->payload - 4);
>Netlink request is using the new nca->data pointer to pass the data as the request payload
>is not the same size and some command payload is bigger than 16 bytes.
>Will you consider to use the same data pointer? So, we don't need to have a checking here.
>If the command is used less than 16 bytes, we can simply assigned &nca->bytes[0] to it.
To keep original structure, we can change 16 bytes to MAX_DATA_LEN. Or I don't see any issue in
Copying data from data pointer from nca but user needs to be aware if it is less than 16 bytes then
use bytes or use data pointer. To keep it simple, we should simply define MAX_LEN.
> > diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
> > index 91b4b66438df..1f338386810d 100644
> > --- a/net/ncsi/ncsi-pkt.h
> > +++ b/net/ncsi/ncsi-pkt.h
> > @@ -151,6 +151,22 @@ struct ncsi_cmd_snfc_pkt {
> > unsigned char pad[22];
> > };
> >
> > +/* OEM Request Command as per NCSI Specification */
> > +struct ncsi_cmd_oem_pkt {
> > + struct ncsi_cmd_pkt_hdr cmd; /* Command header */
> > + __be32 mfr_id; /* Manufacture ID */
> > + unsigned char data[64]; /* OEM Payload Data */
> > + __be32 checksum; /* Checksum */
> > +};
> > +
> > +/* OEM Response Packet as per NCSI Specification */
> > +struct ncsi_rsp_oem_pkt {
> > + struct ncsi_rsp_pkt_hdr rsp; /* Command header */
> > + __be32 mfr_id; /* Manufacture ID */
> > + unsigned char data[64]; /* Payload data */
> > + __be32 checksum; /* Checksum */
> > +};
> > +
>OEM command doesn't not have the fixed response data size too.
>Should we use pointer instead?
Here also we can define MAX_DATA_LEN because data pointer won't map to skb directly.
^ permalink raw reply
* Re: [PATCH v2 0/5] Introducing ixgbe AF_XDP ZC support
From: William Tu @ 2018-10-02 18:43 UTC (permalink / raw)
To: Björn Töpel
Cc: Björn Töpel, jeffrey.t.kirsher, intel-wired-lan,
Karlsson, Magnus, Magnus Karlsson, Alexei Starovoitov,
Daniel Borkmann, Linux Kernel Network Developers,
Jesper Dangaard Brouer, Test, Jakub Kicinski
In-Reply-To: <b507fced-01ab-c109-a7b3-4ed302ca1c00@intel.com>
On Tue, Oct 2, 2018 at 11:39 AM Björn Töpel <bjorn.topel@intel.com> wrote:
>
> On 2018-10-02 20:23, William Tu wrote:
> > On Tue, Oct 2, 2018 at 1:01 AM Björn Töpel <bjorn.topel@gmail.com> wrote:
> >>
> >> From: Björn Töpel <bjorn.topel@intel.com>
> >>
> >> Jeff: Please remove the v1 patches from your dev-queue!
> >>
> >> This patch set introduces zero-copy AF_XDP support for Intel's ixgbe
> >> driver.
> >>
> >> The ixgbe zero-copy code is located in its own file ixgbe_xsk.[ch],
> >> analogous to the i40e ZC support. Again, as in i40e, code paths have
> >> been copied from the XDP path to the zero-copy path. Going forward we
> >> will try to generalize more code between the AF_XDP ZC drivers, and
> >> also reduce the heavy C&P.
> >>
> >> We have run some benchmarks on a dual socket system with two Broadwell
> >> E5 2660 @ 2.0 GHz with hyperthreading turned off. Each socket has 14
> >> cores which gives a total of 28, but only two cores are used in these
> >> experiments. One for TR/RX and one for the user space application. The
> >> memory is DDR4 @ 2133 MT/s (1067 MHz) and the size of each DIMM is
> >> 8192MB and with 8 of those DIMMs in the system we have 64 GB of total
> >> memory. The compiler used is GCC 7.3.0. The NIC is Intel
> >> 82599ES/X520-2 10Gbit/s using the ixgbe driver.
> >>
> >> Below are the results in Mpps of the 82599ES/X520-2 NIC benchmark runs
> >> for 64B and 1500B packets, generated by a commercial packet generator
> >> HW blasting packets at full 10Gbit/s line rate. The results are with
> >> retpoline and all other spectre and meltdown fixes.
> >>
> >> AF_XDP performance 64B packets:
> >> Benchmark XDP_DRV with zerocopy
> >> rxdrop 14.7
> >> txpush 14.6
> >> l2fwd 11.1
> >>
> >> AF_XDP performance 1500B packets:
> >> Benchmark XDP_DRV with zerocopy
> >> rxdrop 0.8
> >> l2fwd 0.8
> >>
> >> XDP performance on our system as a base line.
> >>
> >> 64B packets:
> >> XDP stats CPU Mpps issue-pps
> >> XDP-RX CPU 16 14.7 0
> >>
> >> 1500B packets:
> >> XDP stats CPU Mpps issue-pps
> >> XDP-RX CPU 16 0.8 0
> >>
> >> The structure of the patch set is as follows:
> >>
> >> Patch 1: Introduce Rx/Tx ring enable/disable functionality
> >> Patch 2: Preparatory patche to ixgbe driver code for RX
> >> Patch 3: ixgbe zero-copy support for RX
> >> Patch 4: Preparatory patch to ixgbe driver code for TX
> >> Patch 5: ixgbe zero-copy support for TX
> >>
> >> Changes since v1:
> >>
> >> * Removed redundant AF_XDP precondition checks, pointed out by
> >> Jakub. Now, the preconditions are only checked at XDP enable time.
> >> * Fixed a crash in the egress path, due to incorrect usage of
> >> ixgbe_ring queue_index member. In v2 a ring_idx back reference is
> >> introduced, and used in favor of queue_index. William reported the
> >> crash, and helped me smoke out the issue. Kudos!
> >
> > Thanks! I tested this series and no more crash.
>
> Thank you for spending time on this!
>
> > The number is pretty good (*without* spectre and meltdown fixes)
> > model name : Intel(R) Xeon(R) CPU E5-2440 v2 @ 1.90GHz, total 16 cores/
> >
> > AF_XDP performance 64B packets:
> > Benchmark XDP_DRV with zerocopy
> > rxdrop 20
> > txpush 18
> > l2fwd 20
Sorry please ignore this number!
It's actually 2Mpps from xdpsock but that's because my sender only sends 2Mpps.
>
> What is 20 here? Given that 14.8Mpps is maximum for 64B@10Gbit/s for
> one queue, is this multiple queues? Is this xdpsock or OvS with AF_XDP?
I'm redoing the experiments with higher traffic rate, will report later..
William
^ permalink raw reply
* Re: [PATCH net-next] netfilter: xt_quota: fix the behavior of xt_quota module
From: Pablo Neira Ayuso @ 2018-10-02 18:43 UTC (permalink / raw)
To: Chenbo Feng
Cc: zenczykowski, Chenbo Feng, netdev, netfilter-devel, kernel-team,
Lorenzo Colitti
In-Reply-To: <CAMOXUJn6c+ybxks9siDn=fJo4mGBYk88LuD17BYnPBAPySufDg@mail.gmail.com>
On Tue, Oct 02, 2018 at 11:28:21AM -0700, Chenbo Feng wrote:
> On Tue, Oct 2, 2018 at 11:15 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> >
> > Hi Chenbo,
> >
> > On Tue, Oct 02, 2018 at 10:45:58AM -0700, Chenbo Feng wrote:
> > > On Tue, Oct 2, 2018 at 3:51 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > [...]
> > > Do you mean the remain field will be zeroed when copying the
> > > xt_quota_info struct out of the kernel? I believe that is decided by
> > > the usersize defined in struct xt_match and this patch set it to the
> > > full struct size. So the whole xt_quota_info struct will be copied
> > > into userspace including the field stores the remaining quota. The
> > > userspace will not be aware of it if the ipatbles is not updated but
> > > it should not modify it as well. I have tested the behavior with
> > > net-next branch and it seems working. Am I missing something
> > > recently updated?
> >
> > Hm, I see, I overlook that your patch removes this:
> >
> > - .usersize = offsetof(struct xt_quota_info, master),
> >
> > BTW, is iptables -D command working with your patch?
> >
> > Telling this because if .usersize is removed, then IIRC userspace
> > compares this new remain field with userspace value and deletion will
> > break.
> >
> > Patch that I was referring before is this one from Willem:
> >
> > commit f32815d21d4d8287336fb9cef4d2d9e0866214c2
> > Author: Willem de Bruijn <willemb@google.com>
> > Date: Mon Jan 2 17:19:40 2017 -0500
> >
> > xtables: add xt_match, xt_target and data copy_to_user functions
> >
> > xt_entry_target, xt_entry_match and their private data may contain
> > kernel data.
> > [...]
> > Private data is defined in xt_match and xt_target. All matches and
> > targets that maintain kernel data store this at the tail of their
> > private structure. Extend xt_match and xt_target with .usersize to
> > limit how many bytes of data are copied. The remainder is cleared.
> >
> > Let me know, thanks !
>
> The delete operation is decided by the userspacesize defined in
> userspace ipatbles. I think it is unrelated to the usersize we talk
> about here. For old userspace iptables, the userspacesize is
> offsetof(struct xt_quota_info, master) so it will not compare the rest
> if the struct. And for new iptables we use offsetof(struct
> xt_quota_info, remain). Either way the userspace does not consider the
> remain field when comparing rules so we can do ipatbles rule deletion
> with or without specifying --remain option or even specify --remain to
> a wrong number. We decide to make it this way since the --remain field
> is changing all the time when there is network traffic going on and
> it's hard to compare the remaining quota for new ipatbles as well.
Thanks for explaining and your patience. Patch looks good then.
Thanks!
^ permalink raw reply
* Re: [net-next 0/9][pull request] 100GbE Intel Wired LAN Driver Updates 2018-10-02
From: David Miller @ 2018-10-02 18:41 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, nhorman, sassmann, jogreene
In-Reply-To: <20181002152447.11175-1-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 2 Oct 2018 08:24:38 -0700
> This series contains updates to ice driver only.
If there was a problem yo I'll solve it.
Check out the hook while my DJ revolves it.
Ice ice baby.
Ice ice baby.
Pulled, thanks Jeff.
^ permalink raw reply
* Re: [PATCH v2 0/5] Introducing ixgbe AF_XDP ZC support
From: Björn Töpel @ 2018-10-02 18:39 UTC (permalink / raw)
To: William Tu, Björn Töpel
Cc: jeffrey.t.kirsher, intel-wired-lan, Karlsson, Magnus,
Magnus Karlsson, Alexei Starovoitov, Daniel Borkmann,
Linux Kernel Network Developers, Jesper Dangaard Brouer, Test,
Jakub Kicinski
In-Reply-To: <CALDO+SaKBCFZ5mUum09+FH1s_cEKkqTSSCAZhnFzHXHaNMmxdA@mail.gmail.com>
On 2018-10-02 20:23, William Tu wrote:
> On Tue, Oct 2, 2018 at 1:01 AM Björn Töpel <bjorn.topel@gmail.com> wrote:
>>
>> From: Björn Töpel <bjorn.topel@intel.com>
>>
>> Jeff: Please remove the v1 patches from your dev-queue!
>>
>> This patch set introduces zero-copy AF_XDP support for Intel's ixgbe
>> driver.
>>
>> The ixgbe zero-copy code is located in its own file ixgbe_xsk.[ch],
>> analogous to the i40e ZC support. Again, as in i40e, code paths have
>> been copied from the XDP path to the zero-copy path. Going forward we
>> will try to generalize more code between the AF_XDP ZC drivers, and
>> also reduce the heavy C&P.
>>
>> We have run some benchmarks on a dual socket system with two Broadwell
>> E5 2660 @ 2.0 GHz with hyperthreading turned off. Each socket has 14
>> cores which gives a total of 28, but only two cores are used in these
>> experiments. One for TR/RX and one for the user space application. The
>> memory is DDR4 @ 2133 MT/s (1067 MHz) and the size of each DIMM is
>> 8192MB and with 8 of those DIMMs in the system we have 64 GB of total
>> memory. The compiler used is GCC 7.3.0. The NIC is Intel
>> 82599ES/X520-2 10Gbit/s using the ixgbe driver.
>>
>> Below are the results in Mpps of the 82599ES/X520-2 NIC benchmark runs
>> for 64B and 1500B packets, generated by a commercial packet generator
>> HW blasting packets at full 10Gbit/s line rate. The results are with
>> retpoline and all other spectre and meltdown fixes.
>>
>> AF_XDP performance 64B packets:
>> Benchmark XDP_DRV with zerocopy
>> rxdrop 14.7
>> txpush 14.6
>> l2fwd 11.1
>>
>> AF_XDP performance 1500B packets:
>> Benchmark XDP_DRV with zerocopy
>> rxdrop 0.8
>> l2fwd 0.8
>>
>> XDP performance on our system as a base line.
>>
>> 64B packets:
>> XDP stats CPU Mpps issue-pps
>> XDP-RX CPU 16 14.7 0
>>
>> 1500B packets:
>> XDP stats CPU Mpps issue-pps
>> XDP-RX CPU 16 0.8 0
>>
>> The structure of the patch set is as follows:
>>
>> Patch 1: Introduce Rx/Tx ring enable/disable functionality
>> Patch 2: Preparatory patche to ixgbe driver code for RX
>> Patch 3: ixgbe zero-copy support for RX
>> Patch 4: Preparatory patch to ixgbe driver code for TX
>> Patch 5: ixgbe zero-copy support for TX
>>
>> Changes since v1:
>>
>> * Removed redundant AF_XDP precondition checks, pointed out by
>> Jakub. Now, the preconditions are only checked at XDP enable time.
>> * Fixed a crash in the egress path, due to incorrect usage of
>> ixgbe_ring queue_index member. In v2 a ring_idx back reference is
>> introduced, and used in favor of queue_index. William reported the
>> crash, and helped me smoke out the issue. Kudos!
>
> Thanks! I tested this series and no more crash.
Thank you for spending time on this!
> The number is pretty good (*without* spectre and meltdown fixes)
> model name : Intel(R) Xeon(R) CPU E5-2440 v2 @ 1.90GHz, total 16 cores/
>
> AF_XDP performance 64B packets:
> Benchmark XDP_DRV with zerocopy
> rxdrop 20
> txpush 18
> l2fwd 20
>
What is 20 here? Given that 14.8Mpps is maximum for 64B@10Gbit/s for
one queue, is this multiple queues? Is this xdpsock or OvS with AF_XDP?
Cheers,
Björn
> Regards,
> William
>
^ permalink raw reply
* Re: [PATCH net-next 0/2] qed*: Driver support for 20G link speed.
From: David Miller @ 2018-10-02 18:29 UTC (permalink / raw)
To: sudarsana.kalluru; +Cc: netdev, Michal.Kalderon
In-Reply-To: <20181002131612.10859-1-sudarsana.kalluru@cavium.com>
From: Sudarsana Reddy Kalluru <sudarsana.kalluru@cavium.com>
Date: Tue, 2 Oct 2018 06:16:10 -0700
> From: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
>
> The patch series adds driver support for configuring/reading the 20G link
> speed.
>
> Please consider applying this to "net-next".
Series applied.
^ permalink raw reply
* Re: [PATCH v2 4/5] ixgbe: move common Tx functions to ixgbe_txrx_common.h
From: William Tu @ 2018-10-02 18:28 UTC (permalink / raw)
To: Björn Töpel
Cc: jeffrey.t.kirsher, intel-wired-lan, Björn Töpel,
Karlsson, Magnus, Magnus Karlsson, Alexei Starovoitov,
Daniel Borkmann, Linux Kernel Network Developers,
Jesper Dangaard Brouer, Test, Jakub Kicinski
In-Reply-To: <20181002080034.11754-5-bjorn.topel@gmail.com>
On Tue, Oct 2, 2018 at 1:01 AM Björn Töpel <bjorn.topel@gmail.com> wrote:
>
> From: Björn Töpel <bjorn.topel@intel.com>
>
> This patch prepares for the upcoming zero-copy Tx functionality by
> moving common functions used both by the regular path and zero-copy
> path.
>
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
> ---
Thanks!
Tested-by: William Tu <u9012063@gmail.com>
^ permalink raw reply
* Re: [PATCH net-next] netfilter: xt_quota: fix the behavior of xt_quota module
From: Chenbo Feng @ 2018-10-02 18:28 UTC (permalink / raw)
To: pablo
Cc: zenczykowski, Chenbo Feng, netdev, netfilter-devel, kernel-team,
Lorenzo Colitti
In-Reply-To: <20181002181546.b6zhizcl6lj2inu2@salvia>
On Tue, Oct 2, 2018 at 11:15 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> Hi Chenbo,
>
> On Tue, Oct 02, 2018 at 10:45:58AM -0700, Chenbo Feng wrote:
> > On Tue, Oct 2, 2018 at 3:51 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> [...]
> > Do you mean the remain field will be zeroed when copying the
> > xt_quota_info struct out of the kernel? I believe that is decided by
> > the usersize defined in struct xt_match and this patch set it to the
> > full struct size. So the whole xt_quota_info struct will be copied
> > into userspace including the field stores the remaining quota. The
> > userspace will not be aware of it if the ipatbles is not updated but
> > it should not modify it as well. I have tested the behavior with
> > net-next branch and it seems working. Am I missing something
> > recently updated?
>
> Hm, I see, I overlook that your patch removes this:
>
> - .usersize = offsetof(struct xt_quota_info, master),
>
> BTW, is iptables -D command working with your patch?
>
> Telling this because if .usersize is removed, then IIRC userspace
> compares this new remain field with userspace value and deletion will
> break.
>
> Patch that I was referring before is this one from Willem:
>
> commit f32815d21d4d8287336fb9cef4d2d9e0866214c2
> Author: Willem de Bruijn <willemb@google.com>
> Date: Mon Jan 2 17:19:40 2017 -0500
>
> xtables: add xt_match, xt_target and data copy_to_user functions
>
> xt_entry_target, xt_entry_match and their private data may contain
> kernel data.
> [...]
> Private data is defined in xt_match and xt_target. All matches and
> targets that maintain kernel data store this at the tail of their
> private structure. Extend xt_match and xt_target with .usersize to
> limit how many bytes of data are copied. The remainder is cleared.
>
> Let me know, thanks !
The delete operation is decided by the userspacesize defined in
userspace ipatbles. I think it is unrelated to the usersize we talk
about here. For old userspace iptables, the userspacesize is
offsetof(struct xt_quota_info, master) so it will not compare the rest
if the struct. And for new iptables we use offsetof(struct
xt_quota_info, remain). Either way the userspace does not consider the
remain field when comparing rules so we can do ipatbles rule deletion
with or without specifying --remain option or even specify --remain to
a wrong number. We decide to make it this way since the --remain field
is changing all the time when there is network traffic going on and
it's hard to compare the remaining quota for new ipatbles as well.
^ permalink raw reply
* Re: [PATCH 1/1] net: fec: fix rare tx timeout
From: David Miller @ 2018-10-02 18:28 UTC (permalink / raw)
To: rickard.andersson; +Cc: fugang.duan, netdev, rickaran
In-Reply-To: <20181002124932.14810-2-rickard.andersson@axis.com>
From: <rickard.andersson@axis.com>
Date: Tue, 2 Oct 2018 14:49:32 +0200
> From: Rickard x Andersson <rickaran@axis.com>
>
> During certain heavy network loads TX could time out
> with TX ring dump.
> TX is sometimes never restarted after reaching
> "tx_stop_threshold" because function "fec_enet_tx_queue"
> only tests the first queue.
>
> In addition the TX timeout callback function failed to
> recover because it also operated only on the first queue.
>
> Signed-off-by: Rickard x Andersson <rickaran@axis.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH v2 2/5] ixgbe: move common Rx functions to ixgbe_txrx_common.h
From: William Tu @ 2018-10-02 18:27 UTC (permalink / raw)
To: Björn Töpel
Cc: jeffrey.t.kirsher, intel-wired-lan, Björn Töpel,
Karlsson, Magnus, Magnus Karlsson, Alexei Starovoitov,
Daniel Borkmann, Linux Kernel Network Developers,
Jesper Dangaard Brouer, Test, Jakub Kicinski
In-Reply-To: <20181002080034.11754-3-bjorn.topel@gmail.com>
On Tue, Oct 2, 2018 at 1:01 AM Björn Töpel <bjorn.topel@gmail.com> wrote:
>
> From: Björn Töpel <bjorn.topel@intel.com>
>
> This patch prepares for the upcoming zero-copy Rx functionality, by
> moving/changing linkage of common functions, used both by the regular
> path and zero-copy path.
>
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
> ---
Thanks!
Tested-by: William Tu <u9012063@gmail.com>
^ 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