* Re: [PATCH net-next v7] openvswitch: enable NSH support
From: Yang, Yi @ 2017-09-05 6:37 UTC (permalink / raw)
To: Jiri Benc
Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, e@erig.me,
davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org
In-Reply-To: <20170904124216.6db68e8c@griffin>
On Mon, Sep 04, 2017 at 06:42:16PM +0800, Jiri Benc wrote:
> On Mon, 4 Sep 2017 16:00:05 +0800, Yang, Yi wrote:
> > how can we know next push_nsh uses the same nsh header as previous
> > one?
>
> We store the prepopulated header together with the action.
>
I checked source code but can't find where we can prepopulate it and how
we can deliver the prepopulated header to push_nsh, can you expand it in
order that I can know how to do this in code level?
^ permalink raw reply
* Re: [PATCH] geneve: Fix setting ttl value in collect metadata mode
From: Pravin Shelar @ 2017-09-05 6:47 UTC (permalink / raw)
To: Haishuang Yan
Cc: David S. Miller, Girish Moodalbail,
Linux Kernel Network Developers, linux-kernel
In-Reply-To: <1504442982-12431-1-git-send-email-yanhaishuang@cmss.chinamobile.com>
On Sun, Sep 3, 2017 at 5:49 AM, Haishuang Yan
<yanhaishuang@cmss.chinamobile.com> wrote:
> If key->tos is zero in collect metadata mode, tos should fallback to
> ip{4,6}_dst_hoplimit, same as normal mode.
>
> Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
> ---
> drivers/net/geneve.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index f640407..d52a65f 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -834,11 +834,10 @@ static int geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev,
> sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true);
> if (geneve->collect_md) {
> tos = ip_tunnel_ecn_encap(key->tos, ip_hdr(skb), skb);
> - ttl = key->ttl;
> } else {
> tos = ip_tunnel_ecn_encap(fl4.flowi4_tos, ip_hdr(skb), skb);
> - ttl = key->ttl ? : ip4_dst_hoplimit(&rt->dst);
> }
> + ttl = key->ttl ? : ip4_dst_hoplimit(&rt->dst);
In collect md mode, geneve has to set TTL value from tunnel metadata.
That is the API exposed to userspace. is there reason for this change?
> df = key->tun_flags & TUNNEL_DONT_FRAGMENT ? htons(IP_DF) : 0;
>
> err = geneve_build_skb(&rt->dst, skb, info, xnet, sizeof(struct iphdr));
> @@ -873,12 +872,11 @@ static int geneve6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
> sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true);
> if (geneve->collect_md) {
> prio = ip_tunnel_ecn_encap(key->tos, ip_hdr(skb), skb);
> - ttl = key->ttl;
> } else {
> prio = ip_tunnel_ecn_encap(ip6_tclass(fl6.flowlabel),
> ip_hdr(skb), skb);
> - ttl = key->ttl ? : ip6_dst_hoplimit(dst);
> }
> + ttl = key->ttl ? : ip6_dst_hoplimit(dst);
> err = geneve_build_skb(dst, skb, info, xnet, sizeof(struct ipv6hdr));
> if (unlikely(err))
> return err;
> --
> 1.8.3.1
>
>
>
^ permalink raw reply
* Re: 915f3e3f76 ("mac80211_hwsim: Replace bogus hrtimer clockid"): BUG: kernel reboot-without-warning in test stage
From: Thomas Gleixner @ 2017-09-05 7:12 UTC (permalink / raw)
To: kernel test robot; +Cc: LKP, linux-kernel, netdev, linux-wireless, wfg
In-Reply-To: <59ade247.WzhbxWeytBOa6Xu4%fengguang.wu@intel.com>
On Tue, 5 Sep 2017, kernel test robot wrote:
> Greetings,
>
> 0day kernel testing robot got the below dmesg and the first bad commit is
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
>
> commit 915f3e3f76c05b2da93c4cc278eebc2d9219d9f4
> Author: Thomas Gleixner <tglx@linutronix.de>
> AuthorDate: Sat Feb 25 11:27:37 2017 +0100
> Commit: Linus Torvalds <torvalds@linux-foundation.org>
> CommitDate: Sat Feb 25 09:48:16 2017 -0800
>
> mac80211_hwsim: Replace bogus hrtimer clockid
>
> mac80211_hwsim initializes a hrtimer with clockid CLOCK_MONOTONIC_RAW.
> That's not supported.
>
> Use CLOCK_MONOTNIC instead.
Sorry, no. That bisect is completely bogus. The commit in question merily
replaces the unsupported clockid with a valid one.
Thanks,
tglx
^ permalink raw reply
* Re: [RFC net-next 0/5] TSN: Add qdisc-based config interfaces for traffic shapers
From: Richard Cochran @ 2017-09-05 7:20 UTC (permalink / raw)
To: Jesus Sanchez-Palencia
Cc: Vinicius Costa Gomes, netdev, jhs, xiyou.wangcong, jiri,
intel-wired-lan, andre.guedes, ivan.briano, boon.leong.ong
In-Reply-To: <1519366e-5418-4dda-db00-5bf50a1c67c4@intel.com>
On Fri, Sep 01, 2017 at 09:12:17AM -0700, Jesus Sanchez-Palencia wrote:
> On 09/01/2017 06:03 AM, Richard Cochran wrote:
> > The timing of this RFC is good, as I am just finishing up an RFC that
> > implements time-based transmit using the i210. I'll try and get that
> > out ASAP.
I have an RFC series ready for net-next, but the the merge window just
started. I'll post it when the window closes again...
Thanks,
Richard
^ permalink raw reply
* [PATCH] net/ncsi: fix ncsi_vlan_rx_{add,kill}_vid references
From: Arnd Bergmann @ 2017-09-05 8:05 UTC (permalink / raw)
To: David S. Miller, Gavin Shan, Samuel Mendoza-Jonas
Cc: Arnd Bergmann, Joel Stanley, netdev, linux-kernel
We get a new link error in allmodconfig kernels after ftgmac100
started using the ncsi helpers:
ERROR: "ncsi_vlan_rx_kill_vid" [drivers/net/ethernet/faraday/ftgmac100.ko] undefined!
ERROR: "ncsi_vlan_rx_add_vid" [drivers/net/ethernet/faraday/ftgmac100.ko] undefined!
Related to that, we get another error when CONFIG_NET_NCSI is disabled:
drivers/net/ethernet/faraday/ftgmac100.c:1626:25: error: 'ncsi_vlan_rx_add_vid' undeclared here (not in a function); did you mean 'ncsi_start_dev'?
drivers/net/ethernet/faraday/ftgmac100.c:1627:26: error: 'ncsi_vlan_rx_kill_vid' undeclared here (not in a function); did you mean 'ncsi_vlan_rx_add_vid'?
This fixes both problems at once, using a 'static inline' stub helper
for the disabled case, and exporting the functions when they are present.
Fixes: 51564585d8c6 ("ftgmac100: Support NCSI VLAN filtering when available")
Fixes: 21acf63013ed ("net/ncsi: Configure VLAN tag filter")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
include/net/ncsi.h | 10 ++++++++++
net/ncsi/ncsi-manage.c | 2 ++
2 files changed, 12 insertions(+)
diff --git a/include/net/ncsi.h b/include/net/ncsi.h
index 1f96af46df49..fdc60ff2511d 100644
--- a/include/net/ncsi.h
+++ b/include/net/ncsi.h
@@ -36,6 +36,16 @@ int ncsi_start_dev(struct ncsi_dev *nd);
void ncsi_stop_dev(struct ncsi_dev *nd);
void ncsi_unregister_dev(struct ncsi_dev *nd);
#else /* !CONFIG_NET_NCSI */
+static inline int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16 proto, u16 vid)
+{
+ return -EINVAL;
+}
+
+static inline int ncsi_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, u16 vid)
+{
+ return -EINVAL;
+}
+
static inline struct ncsi_dev *ncsi_register_dev(struct net_device *dev,
void (*notifier)(struct ncsi_dev *nd))
{
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 11904b3b702d..3fd3c39e6278 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -1453,6 +1453,7 @@ int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16 proto, u16 vid)
return found ? ncsi_process_next_channel(ndp) : 0;
}
+EXPORT_SYMBOL_GPL(ncsi_vlan_rx_add_vid);
int ncsi_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, u16 vid)
{
@@ -1491,6 +1492,7 @@ int ncsi_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, u16 vid)
return found ? ncsi_process_next_channel(ndp) : 0;
}
+EXPORT_SYMBOL_GPL(ncsi_vlan_rx_kill_vid);
struct ncsi_dev *ncsi_register_dev(struct net_device *dev,
void (*handler)(struct ncsi_dev *ndev))
--
2.9.0
^ permalink raw reply related
* Re: [PATCH net-next] bpf: fix numa_node validation
From: Daniel Borkmann @ 2017-09-05 8:14 UTC (permalink / raw)
To: Eric Dumazet, David Miller; +Cc: netdev, Martin KaFai Lau, Alexei Starovoitov
In-Reply-To: <1504590062.15310.36.camel@edumazet-glaptop3.roam.corp.google.com>
On 09/05/2017 07:41 AM, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> syzkaller reported crashes in bpf map creation or map update [1]
>
> Problem is that nr_node_ids is a signed integer,
> NUMA_NO_NODE is also an integer, so it is very tempting
> to declare numa_node as a signed integer.
>
> This means the typical test to validate a user provided value :
>
> if (numa_node != NUMA_NO_NODE &&
> (numa_node >= nr_node_ids ||
> !node_online(numa_node)))
>
> must be written :
>
> if (numa_node != NUMA_NO_NODE &&
> ((unsigned int)numa_node >= nr_node_ids ||
> !node_online(numa_node)))
>
>
> [1]
> kernel BUG at mm/slab.c:3256!
> invalid opcode: 0000 [#1] SMP KASAN
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in:
> CPU: 0 PID: 2946 Comm: syzkaller916108 Not tainted 4.13.0-rc7+ #35
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> task: ffff8801d2bc60c0 task.stack: ffff8801c0c90000
> RIP: 0010:____cache_alloc_node+0x1d4/0x1e0 mm/slab.c:3292
> RSP: 0018:ffff8801c0c97638 EFLAGS: 00010096
> RAX: ffffffffffff8b7b RBX: 0000000001080220 RCX: 0000000000000000
> RDX: 00000000ffff8b7b RSI: 0000000001080220 RDI: ffff8801dac00040
> RBP: ffff8801c0c976c0 R08: 0000000000000000 R09: 0000000000000000
> R10: ffff8801c0c97620 R11: 0000000000000001 R12: ffff8801dac00040
> R13: ffff8801dac00040 R14: 0000000000000000 R15: 00000000ffff8b7b
> FS: 0000000002119940(0000) GS:ffff8801db200000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000020001fec CR3: 00000001d2980000 CR4: 00000000001406f0
> Call Trace:
> __do_kmalloc_node mm/slab.c:3688 [inline]
> __kmalloc_node+0x33/0x70 mm/slab.c:3696
> kmalloc_node include/linux/slab.h:535 [inline]
> alloc_htab_elem+0x2a8/0x480 kernel/bpf/hashtab.c:740
> htab_map_update_elem+0x740/0xb80 kernel/bpf/hashtab.c:820
> map_update_elem kernel/bpf/syscall.c:587 [inline]
> SYSC_bpf kernel/bpf/syscall.c:1468 [inline]
> SyS_bpf+0x20c5/0x4c40 kernel/bpf/syscall.c:1443
> entry_SYSCALL_64_fastpath+0x1f/0xbe
> RIP: 0033:0x440409
> RSP: 002b:00007ffd1f1792b8 EFLAGS: 00000246 ORIG_RAX: 0000000000000141
> RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000440409
> RDX: 0000000000000020 RSI: 0000000020006000 RDI: 0000000000000002
> RBP: 0000000000000086 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000401d70
> R13: 0000000000401e00 R14: 0000000000000000 R15: 0000000000000000
> Code: 83 c2 01 89 50 18 4c 03 70 08 e8 38 f4 ff ff 4d 85 f6 0f 85 3e ff ff ff 44 89 fe 4c 89 ef e8 94 fb ff ff 49 89 c6 e9 2b ff ff ff <0f> 0b 0f 0b 0f 0b 66 0f 1f 44 00 00 55 48 89 e5 41 57 41 56 41
> RIP: ____cache_alloc_node+0x1d4/0x1e0 mm/slab.c:3292 RSP: ffff8801c0c97638
> ---[ end trace d745f355da2e33ce ]---
> Kernel panic - not syncing: Fatal exception
>
> Fixes: 96eabe7a40aa ("bpf: Allow selecting numa node during map creation")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Martin KaFai Lau <kafai@fb.com>
> Cc: Alexei Starovoitov <ast@fb.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
Yeah, thanks for catching this, Eric!
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply
* [PATCH] soc: ti/knav_dma: include dmaengine header
From: Arnd Bergmann @ 2017-09-05 8:31 UTC (permalink / raw)
To: arm, David S. Miller
Cc: Dave Jiang, netdev, Arnd Bergmann, Karicheri, Muralidharan,
linux-kernel
A header file cleanup apparently caused a build regression
with one driver using the knav infrastructure:
In file included from drivers/net/ethernet/ti/netcp_core.c:30:0:
include/linux/soc/ti/knav_dma.h:129:30: error: field 'direction' has incomplete type
enum dma_transfer_direction direction;
^~~~~~~~~
drivers/net/ethernet/ti/netcp_core.c: In function 'netcp_txpipe_open':
drivers/net/ethernet/ti/netcp_core.c:1349:21: error: 'DMA_MEM_TO_DEV' undeclared (first use in this function); did you mean 'DMA_MEMORY_MAP'?
config.direction = DMA_MEM_TO_DEV;
^~~~~~~~~~~~~~
DMA_MEMORY_MAP
drivers/net/ethernet/ti/netcp_core.c:1349:21: note: each undeclared identifier is reported only once for each function it appears in
drivers/net/ethernet/ti/netcp_core.c: In function 'netcp_setup_navigator_resources':
drivers/net/ethernet/ti/netcp_core.c:1659:22: error: 'DMA_DEV_TO_MEM' undeclared (first use in this function); did you mean 'DMA_DESC_HOST'?
config.direction = DMA_DEV_TO_MEM;
As the header is no longer included implicitly through netdevice.h,
we should include it in the header that references the enum.
Fixes: 0dd5759dbb1c ("net: remove dmaengine.h inclusion from netdevice.h")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
If the cleanup patch hasn't been submitted for mainline yet, please
add this fixup to the net-next tree, otherwise I'll merge it through
arm-soc.
---
include/linux/soc/ti/knav_dma.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/soc/ti/knav_dma.h b/include/linux/soc/ti/knav_dma.h
index 2b7882666ef6..66693bc4c6ad 100644
--- a/include/linux/soc/ti/knav_dma.h
+++ b/include/linux/soc/ti/knav_dma.h
@@ -17,6 +17,8 @@
#ifndef __SOC_TI_KEYSTONE_NAVIGATOR_DMA_H__
#define __SOC_TI_KEYSTONE_NAVIGATOR_DMA_H__
+#include <linux/dmaengine.h>
+
/*
* PKTDMA descriptor manipulation macros for host packet descriptor
*/
--
2.9.0
^ permalink raw reply related
* [PATCH net] ip6_gre: update mtu properly in ip6gre_err
From: Xin Long @ 2017-09-05 9:26 UTC (permalink / raw)
To: network dev; +Cc: davem
Now when probessing ICMPV6_PKT_TOOBIG, ip6gre_err only subtracts the
offset of gre header from mtu info. The expected mtu of gre device
should also subtract gre header. Otherwise, the next packets still
can't be sent out.
Jianlin found this issue when using the topo:
client(ip6gre)<---->(nic1)route(nic2)<----->(ip6gre)server
and reducing nic2's mtu, then both tcp and sctp's performance with
big size data became 0.
This patch is to fix it by also subtracting grehdr (tun->tun_hlen)
from mtu info when updating gre device's mtu in ip6gre_err(). It
also needs to subtract ETH_HLEN if gre dev'type is ARPHRD_ETHER.
Reported-by: Jianlin Shi <jishi@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/ipv6/ip6_gre.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 67ff2aa..b7a72d4 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -432,7 +432,9 @@ static void ip6gre_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
}
break;
case ICMPV6_PKT_TOOBIG:
- mtu = be32_to_cpu(info) - offset;
+ mtu = be32_to_cpu(info) - offset - t->tun_hlen;
+ if (t->dev->type == ARPHRD_ETHER)
+ mtu -= ETH_HLEN;
if (mtu < IPV6_MIN_MTU)
mtu = IPV6_MIN_MTU;
t->dev->mtu = mtu;
--
2.1.0
^ permalink raw reply related
* Re: [PATCH net-next v7] openvswitch: enable NSH support
From: Jiri Benc @ 2017-09-05 9:46 UTC (permalink / raw)
To: Yang, Yi
Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, e@erig.me,
davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org
In-Reply-To: <20170905055144.GA88800-re2EX8HDrk21gSHoDXDV2kEOCMrvLtNR@public.gmane.org>
On Tue, 5 Sep 2017 13:51:45 +0800, Yang, Yi wrote:
> But if we follow your way, how does nla_for_each_nested handle such
> pattern?
>
> attribute1
> attribute1_mask
> attribute2
> attribute2_mask
> attribute3
> attribute3_mask
Uh? That will just work. Note that nla len contains the size of the
whole attribute, i.e. includes both fields.
> I don't think this can increase performance and readability.
Do you realize you're stating that not copying data around in hot path
can't increase performance? ;-)
> if we use one function to handle both attributes and masks, I can't
> see any substantial difference between two ways as far as the
> performance is concerned.
Except that what you did is so unexpected to netlink that you had to go
out of your way to parse it. Those two memcpys speak for themselves.
> If we consider OVS_KEY_ATTR_NSH as a big attribute, current way is
> precisely following current design pattern
Do you have an idea how nested netlink attributes work? It's very well
defined. What you're doing is breaking the definition. You can't do
that.
The (non-nested) attributes contain header followed by data. The data
is a single value or it is a struct. In ovs for masked actions,
attributes contain a struct of two fields (value and mask). The struct
access is open coded but it's still a struct.
Now, nested attributes are very well defined: it's a nla header
followed by a stream of attributes. There's no requirement on the order
of the attributes. Do you see how you're breaking this assumption? You
expect that each attribute is present exactly twice, once among first
half of attributes, once among second half of attributes. You don't
check that this weird assumption is adhered to. You don't even check
that the point where you split the data is between attributes! You may
very well be splitting an attribute in half and interpreting its data
as nla headers.
Consider your proposal NACKed from my side.
Jiri
^ permalink raw reply
* Re: [PATCH net-next v7] openvswitch: enable NSH support
From: Jiri Benc @ 2017-09-05 9:47 UTC (permalink / raw)
To: Yang, Yi
Cc: netdev@vger.kernel.org, davem@davemloft.net, dev@openvswitch.org,
e@erig.me, blp@ovn.org, jan.scheurich@ericsson.com
In-Reply-To: <20170905063705.GA89551@cran64.bj.intel.com>
On Tue, 5 Sep 2017 14:37:05 +0800, Yang, Yi wrote:
> I checked source code but can't find where we can prepopulate it and how
> we can deliver the prepopulated header to push_nsh, can you expand it in
> order that I can know how to do this in code level?
I already said that it's not implemented yet and can be done as a
future performance enhancement.
Jiri
^ permalink raw reply
* Re: [PATCH net-next v7] openvswitch: enable NSH support
From: Jiri Benc @ 2017-09-05 9:49 UTC (permalink / raw)
To: Jan Scheurich
Cc: Yang, Yi, netdev@vger.kernel.org, davem@davemloft.net,
dev@openvswitch.org, e@erig.me, blp@ovn.org
In-Reply-To: <CFF8EF42F1132E4CBE2BF0AB6C21C58D787F4F69@ESESSMB107.ericsson.se>
On Mon, 4 Sep 2017 14:45:50 +0000, Jan Scheurich wrote:
> So what is the correct layout for MASKED_SET action with nested fields?
> 1. All nested values, followed by all nested masks, or
> 2. For each nested field value followed by mask?
>
> I guess alternative 1, but just to be sure.
It's 2. Alternative 1 breaks netlink assumptions, is ugly to implement
and probably impossible to be properly validated.
Jiri
^ permalink raw reply
* Re: [PATCH net-next v6 3/3] openvswitch: enable NSH support
From: Hannes Frederic Sowa @ 2017-09-05 10:30 UTC (permalink / raw)
To: Yang, Yi
Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
jbenc-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, e@erig.me
In-Reply-To: <20170905021112.GA86057-re2EX8HDrk21gSHoDXDV2kEOCMrvLtNR@public.gmane.org>
"Yang, Yi" <yi.y.yang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> writes:
> I'm not sure what new action you expect to bring here, I think group
> action is just for this, as you said it isn't only bound to NSH, you can
> start a new thread to discuss this. I don't think it is in scope of NSH.
It is in scope of this discussion as you will provide a user space API
that makes the NSH context fields accessible from user space in a
certain way. If you commit to this, there is no way going back.
I haven't yet grasped the idea on how those fields will be used in OVS
besides load balancing. Even for load balancing the tunnel itself
(vxlan-gpe + UDP source port or ipv6 flowlabel) already provides enough
entropy to do per-flow load balancing. What else is needed? Why a
context header for that? You just need multiple action chains and pick
one randomly.
The only protocol that I can compare that to is geneve with TLVs, but
the TLVs are global and uniquie and a property of the networking
forwarding backplane and not a property of the path inside a tenant. So
I expect this actually to be the first case where I think that matters.
Why are context labels that special that they are not part of tun_ops?
Thanks,
Hannes
^ permalink raw reply
* Re: [PATCH net-next v7] openvswitch: enable NSH support
From: Jan Scheurich @ 2017-09-05 10:36 UTC (permalink / raw)
To: Jiri Benc
Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, e@erig.me,
davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org
In-Reply-To: <20170905114949.5849b75f@griffin>
> From: Jiri Benc [mailto:jbenc-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org]
> > So what is the correct layout for MASKED_SET action with nested fields?
> > 1. All nested values, followed by all nested masks, or
> > 2. For each nested field value followed by mask?
> >
> > I guess alternative 1, but just to be sure.
>
> It's 2. Alternative 1 breaks netlink assumptions, is ugly to implement
> and probably impossible to be properly validated.
OK. For outsiders this was far from obvious :-)
So, for OVS_ACTION_ATTR_SET_MASKED any nested attribute, no matter on which nesting level, must contain value directly followed by mask.
If that is the principle of handling masks in Netlink APIs, than we must adhere to it.
BR, Jan
^ permalink raw reply
* [PATCH net] net: sched: don't use GFP_KERNEL under spin lock
From: Jakub Kicinski @ 2017-09-05 10:54 UTC (permalink / raw)
To: netdev; +Cc: jiri, Chris Mi, xiyou.wangcong, jhs, oss-drivers, Jakub Kicinski
The new TC IDR code uses GFP_KERNEL under spinlocks. Which leads
to:
[ 582.621091] BUG: sleeping function called from invalid context at ../mm/slab.h:416
[ 582.629721] in_atomic(): 1, irqs_disabled(): 0, pid: 3379, name: tc
[ 582.636939] 2 locks held by tc/3379:
[ 582.641049] #0: (rtnl_mutex){+.+.+.}, at: [<ffffffff910354ce>] rtnetlink_rcv_msg+0x92e/0x1400
[ 582.650958] #1: (&(&tn->idrinfo->lock)->rlock){+.-.+.}, at: [<ffffffff9110a5e0>] tcf_idr_create+0x2f0/0x8e0
[ 582.662217] Preemption disabled at:
[ 582.662222] [<ffffffff9110a5e0>] tcf_idr_create+0x2f0/0x8e0
[ 582.672592] CPU: 9 PID: 3379 Comm: tc Tainted: G W 4.13.0-rc7-debug-00648-g43503a79b9f0 #287
[ 582.683432] Hardware name: Dell Inc. PowerEdge R730/072T6D, BIOS 2.3.4 11/08/2016
[ 582.691937] Call Trace:
...
[ 582.713332] ? tcf_idr_create+0x2f0/0x8e0
[ 582.717925] ___might_sleep+0x40f/0x660
[ 582.722336] ? finish_task_switch+0xb90/0xb90
[ 582.727315] ? mark_held_locks+0xdd/0x190
[ 582.731908] __might_sleep+0xba/0x240
[ 582.736125] ? radix_tree_node_alloc.constprop.6+0x4a/0x450
[ 582.742460] kmem_cache_alloc+0x286/0x540
[ 582.747055] radix_tree_node_alloc.constprop.6+0x4a/0x450
[ 582.753209] idr_get_free_cmn+0x627/0xf80
...
[ 582.815525] idr_alloc_cmn+0x1a8/0x270
[ 582.819821] ? __raw_spin_lock_init+0x21/0x120
[ 582.824914] ? idr_replace+0x20/0x20
[ 582.829013] ? do_raw_spin_lock+0x1f0/0x1f0
[ 582.833804] tcf_idr_create+0x31b/0x8e0
...
Fixes: 65a206c01e8e ("net/sched: Change act_api and act_xxx modules to use IDR")
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
net/sched/act_api.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 0eb545bcb247..a48e4b45722d 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -298,7 +298,7 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
if (!index) {
spin_lock_bh(&idrinfo->lock);
err = idr_alloc_ext(idr, NULL, &idr_index, 1, 0,
- GFP_KERNEL);
+ GFP_ATOMIC);
spin_unlock_bh(&idrinfo->lock);
if (err) {
err3:
@@ -309,7 +309,7 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
} else {
spin_lock_bh(&idrinfo->lock);
err = idr_alloc_ext(idr, NULL, NULL, index, index + 1,
- GFP_KERNEL);
+ GFP_ATOMIC);
spin_unlock_bh(&idrinfo->lock);
if (err)
goto err3;
--
2.14.1
^ permalink raw reply related
* Re: [PATCH net-next v7] openvswitch: enable NSH support
From: Yang, Yi @ 2017-09-05 10:57 UTC (permalink / raw)
To: Jiri Benc
Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, e@erig.me,
davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org
In-Reply-To: <20170905114741.537dfc11@griffin>
On Tue, Sep 05, 2017 at 11:47:41AM +0200, Jiri Benc wrote:
> On Tue, 5 Sep 2017 14:37:05 +0800, Yang, Yi wrote:
> > I checked source code but can't find where we can prepopulate it and how
> > we can deliver the prepopulated header to push_nsh, can you expand it in
> > order that I can know how to do this in code level?
>
> I already said that it's not implemented yet and can be done as a
> future performance enhancement.
Got it, thanks.
>
> Jiri
^ permalink raw reply
* Re: [PATCH net] net: sched: don't use GFP_KERNEL under spin lock
From: Jakub Kicinski @ 2017-09-05 10:59 UTC (permalink / raw)
To: netdev; +Cc: jiri, oss-drivers
In-Reply-To: <20170905105442.30972-1-jakub.kicinski@netronome.com>
Jiri, FWIW I also just noticed these, but they seem related to the
earlier TC chain work?
unreferenced object 0xffff8807466f70c8 (size 64):
comm "tc", pid 1812, jiffies 4294932641 (age 1537.192s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 a0 83 e1 31 07 88 ff ff ...........1....
d8 70 6f 46 07 88 ff ff d8 70 6f 46 07 88 ff ff .poF.....poF....
backtrace:
[<ffffffffaa6b0118>] kmemleak_alloc+0x28/0x50
[<ffffffffa89a89b6>] kmem_cache_alloc_trace+0x1e6/0x550
[<ffffffffaa102827>] tcf_chain_create+0x97/0x460
[<ffffffffaa102f67>] tcf_block_get+0x127/0x230
[<ffffffffc03e8546>] ingress_init+0x76/0x110 [sch_ingress]
[<ffffffffaa0fbb1c>] qdisc_create+0x2ec/0x1100
[<ffffffffaa0fce1a>] tc_modify_qdisc+0x4ea/0x1ae0
[<ffffffffaa0355dc>] rtnetlink_rcv_msg+0xa3c/0x1400
[<ffffffffaa12e3cb>] netlink_rcv_skb+0x22b/0x4e0
[<ffffffffaa024b55>] rtnetlink_rcv+0x15/0x20
[<ffffffffaa12bddd>] netlink_unicast+0x47d/0x710
[<ffffffffaa12caef>] netlink_sendmsg+0xa7f/0x1140
[<ffffffffa9f15c52>] sock_sendmsg+0xe2/0x170
[<ffffffffa9f188bd>] ___sys_sendmsg+0x72d/0xcc0
[<ffffffffa9f1bfa2>] __sys_sendmsg+0xe2/0x260
[<ffffffffa9f1c132>] SyS_sendmsg+0x12/0x20
unreferenced object 0xffff88074db6c008 (size 64):
comm "tc", pid 2097, jiffies 4294953636 (age 1453.212s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 60 a6 d1 31 07 88 ff ff ........`..1....
18 c0 b6 4d 07 88 ff ff 18 c0 b6 4d 07 88 ff ff ...M.......M....
backtrace:
[<ffffffffaa6b0118>] kmemleak_alloc+0x28/0x50
[<ffffffffa89a89b6>] kmem_cache_alloc_trace+0x1e6/0x550
[<ffffffffaa102827>] tcf_chain_create+0x97/0x460
[<ffffffffaa102f67>] tcf_block_get+0x127/0x230
[<ffffffffc03e8546>] ingress_init+0x76/0x110 [sch_ingress]
[<ffffffffaa0fbb1c>] qdisc_create+0x2ec/0x1100
[<ffffffffaa0fce1a>] tc_modify_qdisc+0x4ea/0x1ae0
[<ffffffffaa0355dc>] rtnetlink_rcv_msg+0xa3c/0x1400
[<ffffffffaa12e3cb>] netlink_rcv_skb+0x22b/0x4e0
[<ffffffffaa024b55>] rtnetlink_rcv+0x15/0x20
[<ffffffffaa12bddd>] netlink_unicast+0x47d/0x710
[<ffffffffaa12caef>] netlink_sendmsg+0xa7f/0x1140
[<ffffffffa9f15c52>] sock_sendmsg+0xe2/0x170
[<ffffffffa9f188bd>] ___sys_sendmsg+0x72d/0xcc0
[<ffffffffa9f1bfa2>] __sys_sendmsg+0xe2/0x260
[<ffffffffa9f1c132>] SyS_sendmsg+0x12/0x20
unreferenced object 0xffff880749f82a48 (size 64):
comm "tc", pid 2381, jiffies 4294972986 (age 1375.812s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 c0 b7 31 31 07 88 ff ff ..........11....
58 2a f8 49 07 88 ff ff 58 2a f8 49 07 88 ff ff X*.I....X*.I....
backtrace:
[<ffffffffaa6b0118>] kmemleak_alloc+0x28/0x50
[<ffffffffa89a89b6>] kmem_cache_alloc_trace+0x1e6/0x550
[<ffffffffaa102827>] tcf_chain_create+0x97/0x460
[<ffffffffaa102f67>] tcf_block_get+0x127/0x230
[<ffffffffc03e8546>] ingress_init+0x76/0x110 [sch_ingress]
[<ffffffffaa0fbb1c>] qdisc_create+0x2ec/0x1100
[<ffffffffaa0fce1a>] tc_modify_qdisc+0x4ea/0x1ae0
[<ffffffffaa0355dc>] rtnetlink_rcv_msg+0xa3c/0x1400
[<ffffffffaa12e3cb>] netlink_rcv_skb+0x22b/0x4e0
[<ffffffffaa024b55>] rtnetlink_rcv+0x15/0x20
[<ffffffffaa12bddd>] netlink_unicast+0x47d/0x710
[<ffffffffaa12caef>] netlink_sendmsg+0xa7f/0x1140
[<ffffffffa9f15c52>] sock_sendmsg+0xe2/0x170
[<ffffffffa9f188bd>] ___sys_sendmsg+0x72d/0xcc0
[<ffffffffa9f1bfa2>] __sys_sendmsg+0xe2/0x260
[<ffffffffa9f1c132>] SyS_sendmsg+0x12/0x20
unreferenced object 0xffff8803fd855a08 (size 64):
comm "tc", pid 2663, jiffies 4294992152 (age 1299.328s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 20 49 80 31 07 88 ff ff ........ I.1....
18 5a 85 fd 03 88 ff ff 18 5a 85 fd 03 88 ff ff .Z.......Z......
backtrace:
[<ffffffffaa6b0118>] kmemleak_alloc+0x28/0x50
[<ffffffffa89a89b6>] kmem_cache_alloc_trace+0x1e6/0x550
[<ffffffffaa102827>] tcf_chain_create+0x97/0x460
[<ffffffffaa102f67>] tcf_block_get+0x127/0x230
[<ffffffffc03e8546>] ingress_init+0x76/0x110 [sch_ingress]
[<ffffffffaa0fbb1c>] qdisc_create+0x2ec/0x1100
[<ffffffffaa0fce1a>] tc_modify_qdisc+0x4ea/0x1ae0
[<ffffffffaa0355dc>] rtnetlink_rcv_msg+0xa3c/0x1400
[<ffffffffaa12e3cb>] netlink_rcv_skb+0x22b/0x4e0
[<ffffffffaa024b55>] rtnetlink_rcv+0x15/0x20
[<ffffffffaa12bddd>] netlink_unicast+0x47d/0x710
[<ffffffffaa12caef>] netlink_sendmsg+0xa7f/0x1140
[<ffffffffa9f15c52>] sock_sendmsg+0xe2/0x170
[<ffffffffa9f188bd>] ___sys_sendmsg+0x72d/0xcc0
[<ffffffffa9f1bfa2>] __sys_sendmsg+0xe2/0x260
[<ffffffffa9f1c132>] SyS_sendmsg+0x12/0x20
unreferenced object 0xffff88071b096f28 (size 64):
comm "tc", pid 2943, jiffies 4295013397 (age 1214.352s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 c0 37 e7 30 07 88 ff ff .........7.0....
38 6f 09 1b 07 88 ff ff 38 6f 09 1b 07 88 ff ff 8o......8o......
backtrace:
[<ffffffffaa6b0118>] kmemleak_alloc+0x28/0x50
[<ffffffffa89a89b6>] kmem_cache_alloc_trace+0x1e6/0x550
[<ffffffffaa102827>] tcf_chain_create+0x97/0x460
[<ffffffffaa102f67>] tcf_block_get+0x127/0x230
[<ffffffffc03e8546>] ingress_init+0x76/0x110 [sch_ingress]
[<ffffffffaa0fbb1c>] qdisc_create+0x2ec/0x1100
[<ffffffffaa0fce1a>] tc_modify_qdisc+0x4ea/0x1ae0
[<ffffffffaa0355dc>] rtnetlink_rcv_msg+0xa3c/0x1400
[<ffffffffaa12e3cb>] netlink_rcv_skb+0x22b/0x4e0
[<ffffffffaa024b55>] rtnetlink_rcv+0x15/0x20
[<ffffffffaa12bddd>] netlink_unicast+0x47d/0x710
[<ffffffffaa12caef>] netlink_sendmsg+0xa7f/0x1140
[<ffffffffa9f15c52>] sock_sendmsg+0xe2/0x170
[<ffffffffa9f188bd>] ___sys_sendmsg+0x72d/0xcc0
[<ffffffffa9f1bfa2>] __sys_sendmsg+0xe2/0x260
[<ffffffffa9f1c132>] SyS_sendmsg+0x12/0x20
unreferenced object 0xffff880731f66568 (size 64):
comm "tc", pid 3263, jiffies 4295033366 (age 1134.480s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 95 64 65 07 88 ff ff ..........de....
78 65 f6 31 07 88 ff ff 78 65 f6 31 07 88 ff ff xe.1....xe.1....
backtrace:
[<ffffffffaa6b0118>] kmemleak_alloc+0x28/0x50
[<ffffffffa89a89b6>] kmem_cache_alloc_trace+0x1e6/0x550
[<ffffffffaa102827>] tcf_chain_create+0x97/0x460
[<ffffffffaa102f67>] tcf_block_get+0x127/0x230
[<ffffffffc03e8546>] ingress_init+0x76/0x110 [sch_ingress]
[<ffffffffaa0fbb1c>] qdisc_create+0x2ec/0x1100
[<ffffffffaa0fce1a>] tc_modify_qdisc+0x4ea/0x1ae0
[<ffffffffaa0355dc>] rtnetlink_rcv_msg+0xa3c/0x1400
[<ffffffffaa12e3cb>] netlink_rcv_skb+0x22b/0x4e0
[<ffffffffaa024b55>] rtnetlink_rcv+0x15/0x20
[<ffffffffaa12bddd>] netlink_unicast+0x47d/0x710
[<ffffffffaa12caef>] netlink_sendmsg+0xa7f/0x1140
[<ffffffffa9f15c52>] sock_sendmsg+0xe2/0x170
[<ffffffffa9f188bd>] ___sys_sendmsg+0x72d/0xcc0
[<ffffffffa9f1bfa2>] __sys_sendmsg+0xe2/0x260
[<ffffffffa9f1c132>] SyS_sendmsg+0x12/0x20
^ permalink raw reply
* [PATCH iproute2/master] tc actions: store and dump correct length of user cookies
From: Simon Horman @ 2017-09-05 11:06 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, oss-drivers, Simon Horman, Jamal Hadi Salim
Correct two errors which cancel each other out:
* Do not send twice the length of the actual provided by the user to the kernel
* Do not dump half the length of the cookie provided by the kernel
As the cookie is now stored in the kernel at its correct length rather
than double the that length cookies of up to the maximum size of 16 bytes
may now be stored rather than a maximum of half that length.
Output of dump is the same before and after this change,
but the data stored in the kernel is now exactly the cookie
rather than the cookie + as many trailing zeros.
Before:
# tc filter add dev eth0 protocol ip parent ffff: \
flower ip_proto udp action drop \
cookie 0123456789abcdef0123456789abcdef
RTNETLINK answers: Invalid argument
After:
# tc filter add dev eth0 protocol ip parent ffff: \
flower ip_proto udp action drop \
cookie 0123456789abcdef0123456789abcdef
# tc filter show dev eth0 ingress
eth_type ipv4
ip_proto udp
not_in_hw
action order 1: gact action drop
random type none pass val 0
index 1 ref 1 bind 1 installed 1 sec used 1 sec
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
cookie len 16 0123456789abcdef0123456789abcdef
Fixes: fd8b3d2c1b9b ("actions: Add support for user cookies")
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
tc/m_action.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tc/m_action.c b/tc/m_action.c
index 6ebe85e1cbe3..397e9c5d4a7b 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -242,7 +242,7 @@ done0:
invarg("cookie must be a hex string\n",
*argv);
- act_ck_len = slen;
+ act_ck_len = slen / 2;
argc--;
argv++;
}
@@ -307,7 +307,7 @@ static int tc_print_one_action(FILE *f, struct rtattr *arg)
print_tcstats2_attr(f, tb[TCA_ACT_STATS], "\t", NULL);
if (tb[TCA_ACT_COOKIE]) {
int strsz = RTA_PAYLOAD(tb[TCA_ACT_COOKIE]);
- char b1[strsz+1];
+ char b1[strsz * 2 + 1];
fprintf(f, "\n\tcookie len %d %s ", strsz,
hexstring_n2a(RTA_DATA(tb[TCA_ACT_COOKIE]),
--
2.1.4
^ permalink raw reply related
* Re: [pull request][net-next 0/3] Mellanox, mlx5 GRE tunnel offloads
From: Hannes Frederic Sowa @ 2017-09-05 11:14 UTC (permalink / raw)
To: Tom Herbert
Cc: Saeed Mahameed, Saeed Mahameed, David S. Miller,
Linux Netdev List
In-Reply-To: <CALx6S34aDER1Gzpcw+p7+yr91mjiixCh6gnawKSxy7zhVQz+6Q@mail.gmail.com>
Tom Herbert <tom@herbertland.com> writes:
> There is absolutely no requirement in IP that packets are delivered in
> order-- there never has been and there never will be! If the ULP, like
> Ethernet encapsulation, requires in order deliver then it needs to
> implement that itself like TCP, GRE, and other protocols ensure that
> with sequence numbers and reassembly. All of these hoops we do make
> sure that packets always follow the same path and are always in order
> are done for benefit of middlebox devices like stateful firewalls that
> have force us to adopt their concept of network architecture-- in the
> long run this is self-defeating and kills our ability to innovate.
>
> I'm not saying that we shouldn't consider legacy devices, but we
> should scrutinize new development or solutions that perpetuate
> incorrect design or bad assumptions.
So configure RSS per port and ensure no fragments are send to those
ports. This is possible and rather easy to do. It solves the problem
with legacy software and it spreads out packets for your applications.
It is not perfect but it is working and solves both problems.
Bye,
Hannes
^ permalink raw reply
* Re: [PATCH net-next v7] openvswitch: enable NSH support
From: Yang, Yi @ 2017-09-05 11:03 UTC (permalink / raw)
To: Jiri Benc
Cc: netdev@vger.kernel.org, davem@davemloft.net, dev@openvswitch.org,
e@erig.me, blp@ovn.org, jan.scheurich@ericsson.com
In-Reply-To: <20170905114645.33bf0e32@griffin>
On Tue, Sep 05, 2017 at 11:46:45AM +0200, Jiri Benc wrote:
> On Tue, 5 Sep 2017 13:51:45 +0800, Yang, Yi wrote:
> > But if we follow your way, how does nla_for_each_nested handle such
> > pattern?
> >
> > attribute1
> > attribute1_mask
> > attribute2
> > attribute2_mask
> > attribute3
> > attribute3_mask
>
> Uh? That will just work. Note that nla len contains the size of the
> whole attribute, i.e. includes both fields.
>
> > I don't think this can increase performance and readability.
>
I got the point, actually value and mask are data of one attribute, so
they are three attributes but not six.
^ permalink raw reply
* Re: [PATCH net-next v6 3/3] openvswitch: enable NSH support
From: Yang, Yi @ 2017-09-05 11:38 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
jbenc-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, e@erig.me
In-Reply-To: <87vakxsaj2.fsf-tFNcAqjVMyqKXQKiL6tip0B+6BGkLq7r@public.gmane.org>
On Tue, Sep 05, 2017 at 12:30:09PM +0200, Hannes Frederic Sowa wrote:
> "Yang, Yi" <yi.y.yang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> writes:
>
> > I'm not sure what new action you expect to bring here, I think group
> > action is just for this, as you said it isn't only bound to NSH, you can
> > start a new thread to discuss this. I don't think it is in scope of NSH.
>
> It is in scope of this discussion as you will provide a user space API
> that makes the NSH context fields accessible from user space in a
> certain way. If you commit to this, there is no way going back.
We can change this later if we really find a better way to handle this
because it isn't defined in include/uapi/linux/openvswitch.h, so I still
have backdoor to do this if needed :-)
>
> I haven't yet grasped the idea on how those fields will be used in OVS
> besides load balancing. Even for load balancing the tunnel itself
> (vxlan-gpe + UDP source port or ipv6 flowlabel) already provides enough
> entropy to do per-flow load balancing. What else is needed? Why a
> context header for that? You just need multiple action chains and pick
> one randomly.
For our sfc use case in Opendaylight, we use context[0] for tunnel ID,
context[1] for destination IP for reverse RSP, they are used to match
and set in OpenFlow table, you can't limit users to use them in such
ways.
If you check GENEVE implementation, tun_metadata* can be set or matched
as any other match field.
Actually the most important information in NSH are just these context
headers, you can't limit imagination of users by removing them from flow
keys.
My point is to bring miniflow into kernel data path to fix your concern,
this will benefit your employer directly :-)
I'm just curious your company has hardware to offload NSH? Which
company are you from?
>
> The only protocol that I can compare that to is geneve with TLVs, but
> the TLVs are global and uniquie and a property of the networking
> forwarding backplane and not a property of the path inside a tenant. So
> I expect this actually to be the first case where I think that matters.
>
> Why are context labels that special that they are not part of tun_ops?
>
> Thanks,
> Hannes
^ permalink raw reply
* RE: [PATCH net-next v6 3/3] openvswitch: enable NSH support
From: Jan Scheurich @ 2017-09-05 12:19 UTC (permalink / raw)
To: Hannes Frederic Sowa, Yang, Yi
Cc: netdev@vger.kernel.org, dev@openvswitch.org, jbenc@redhat.com,
e@erig.me, blp@ovn.org
In-Reply-To: <87vakxsaj2.fsf@stressinduktion.org>
Hi Hannes,
Please have a look at the Google doc that sketches the overall solution to support NSH in OVS.
https://drive.google.com/open?id=1oWMYUH8sjZJzWa72o2q9kU0N6pNE-rwZcLH3-kbbDR8
In details it is slightly outdated but the NSH MD1 support (and all prerequisites like PTAP and Generic Encap/Decap) have been implemented in OVS 2.8 (ofproto layer and the userspace datapath).
The NSH use cases are discussed in the chapter "Support for NSH". OVS is primarily targeting the Classifier and SFF use cases. Obviously the classifier must be able to set the MD1 context headers. The final SFF must be able to inspect the context headers, typically to determine the L2 or L3 forwarding context (e.g. VRF) in which to forward the decapsulated packet on to its final destination.
This information has end-to-end significance for the SFP and is encoded by the classifier in the NSH context headers. It cannot be put into transport tunnel options of e.g. a Geneve tunnel connecting two SFFs along the SFP.
We are now trying to establish feature parity in the kernel datapath. If the kernel datapath chooses not to support the MD1 fields, OVS with kernel datapath will not be able to address the classifier and final SFF use cases.
BR, Jan
> -----Original Message-----
> From: Hannes Frederic Sowa [mailto:hannes@stressinduktion.org]
> Sent: Tuesday, 05 September, 2017 12:30
> To: Yang, Yi <yi.y.yang@intel.com>
> Cc: netdev@vger.kernel.org; dev@openvswitch.org; jbenc@redhat.com; e@erig.me; blp@ovn.org; Jan Scheurich
> <jan.scheurich@ericsson.com>
> Subject: Re: [PATCH net-next v6 3/3] openvswitch: enable NSH support
>
> "Yang, Yi" <yi.y.yang@intel.com> writes:
>
> > I'm not sure what new action you expect to bring here, I think group
> > action is just for this, as you said it isn't only bound to NSH, you can
> > start a new thread to discuss this. I don't think it is in scope of NSH.
>
> It is in scope of this discussion as you will provide a user space API
> that makes the NSH context fields accessible from user space in a
> certain way. If you commit to this, there is no way going back.
>
> I haven't yet grasped the idea on how those fields will be used in OVS
> besides load balancing. Even for load balancing the tunnel itself
> (vxlan-gpe + UDP source port or ipv6 flowlabel) already provides enough
> entropy to do per-flow load balancing. What else is needed? Why a
> context header for that? You just need multiple action chains and pick
> one randomly.
>
> The only protocol that I can compare that to is geneve with TLVs, but
> the TLVs are global and uniquie and a property of the networking
> forwarding backplane and not a property of the path inside a tenant. So
> I expect this actually to be the first case where I think that matters.
>
> Why are context labels that special that they are not part of tun_ops?
>
> Thanks,
> Hannes
^ permalink raw reply
* [PATCH v8 05/20] crypto: marvell/cesa: remove redundant backlog checks on EBUSY
From: Gilad Ben-Yossef @ 2017-09-05 12:38 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Tom Lendacky, Gary Hook, Boris Brezillon, Arnaud Ebalard,
Matthias Brugger, Alasdair Kergon, Mike Snitzer, dm-devel,
Shaohua Li, Steve French, Theodore Y. Ts'o, Jaegeuk Kim,
Steffen Klassert, Alexey Kuznetsov, Hideaki YOSHIFUJI, Mimi Zohar,
Dmitry Kasatkin, James Morris, Serge E. Hallyn, linux-crypto,
linux-doc, linux-kernel, keyrings, linux-arm-kernel,
linux-mediatek, linux-raid, linux-cifs, samba-technical,
linux-fscrypt, netdev, linux-ima-devel, linux-ima-user,
linux-security-module
Cc: Ofir Drang
In-Reply-To: <1504615144-29770-1-git-send-email-gilad@benyossef.com>
Now that -EBUSY return code only indicates backlog queueing
we can safely remove the now redundant check for the
CRYPTO_TFM_REQ_MAY_BACKLOG flag when -EBUSY is returned.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
drivers/crypto/marvell/cesa.c | 3 +--
drivers/crypto/marvell/cesa.h | 2 +-
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/marvell/cesa.c b/drivers/crypto/marvell/cesa.c
index 6e7a5c7..0c3909d 100644
--- a/drivers/crypto/marvell/cesa.c
+++ b/drivers/crypto/marvell/cesa.c
@@ -183,8 +183,7 @@ int mv_cesa_queue_req(struct crypto_async_request *req,
spin_lock_bh(&engine->lock);
ret = crypto_enqueue_request(&engine->queue, req);
if ((mv_cesa_req_get_type(creq) == CESA_DMA_REQ) &&
- (ret == -EINPROGRESS ||
- (ret == -EBUSY && req->flags & CRYPTO_TFM_REQ_MAY_BACKLOG)))
+ (ret == -EINPROGRESS || ret == -EBUSY))
mv_cesa_tdma_chain(engine, creq);
spin_unlock_bh(&engine->lock);
diff --git a/drivers/crypto/marvell/cesa.h b/drivers/crypto/marvell/cesa.h
index b7872f6..63c8457 100644
--- a/drivers/crypto/marvell/cesa.h
+++ b/drivers/crypto/marvell/cesa.h
@@ -763,7 +763,7 @@ static inline int mv_cesa_req_needs_cleanup(struct crypto_async_request *req,
* the backlog and will be processed later. There's no need to
* clean it up.
*/
- if (ret == -EBUSY && req->flags & CRYPTO_TFM_REQ_MAY_BACKLOG)
+ if (ret == -EBUSY)
return false;
/* Request wasn't queued, we need to clean it up */
--
2.1.4
^ permalink raw reply related
* [PATCH v8 08/20] crypto: move pub key to generic async completion
From: Gilad Ben-Yossef @ 2017-09-05 12:38 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Tom Lendacky, Gary Hook, Boris Brezillon, Arnaud Ebalard,
Matthias Brugger, Alasdair Kergon, Mike Snitzer, dm-devel,
Shaohua Li, Steve French, Theodore Y. Ts'o, Jaegeuk Kim,
Steffen Klassert, Alexey Kuznetsov, Hideaki YOSHIFUJI, Mimi Zohar,
Dmitry Kasatkin, James Morris, Serge E. Hallyn, linux-crypto,
linux-doc, linux-kernel, keyrings, linux-arm-kernel,
linux-mediatek, linux-raid, linux-cifs, samba-technical,
linux-fscrypt, netdev, linux-ima-devel, linux-ima-user,
linux-security-module
Cc: Ofir Drang
In-Reply-To: <1504615144-29770-1-git-send-email-gilad@benyossef.com>
public_key_verify_signature() is starting an async crypto op and
waiting for it to complete. Move it over to generic code doing
the same.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
crypto/asymmetric_keys/public_key.c | 28 ++++------------------------
1 file changed, 4 insertions(+), 24 deletions(-)
diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index 3cd6e12..d916235 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -57,29 +57,13 @@ static void public_key_destroy(void *payload0, void *payload3)
public_key_signature_free(payload3);
}
-struct public_key_completion {
- struct completion completion;
- int err;
-};
-
-static void public_key_verify_done(struct crypto_async_request *req, int err)
-{
- struct public_key_completion *compl = req->data;
-
- if (err == -EINPROGRESS)
- return;
-
- compl->err = err;
- complete(&compl->completion);
-}
-
/*
* Verify a signature using a public key.
*/
int public_key_verify_signature(const struct public_key *pkey,
const struct public_key_signature *sig)
{
- struct public_key_completion compl;
+ struct crypto_wait cwait;
struct crypto_akcipher *tfm;
struct akcipher_request *req;
struct scatterlist sig_sg, digest_sg;
@@ -131,20 +115,16 @@ int public_key_verify_signature(const struct public_key *pkey,
sg_init_one(&digest_sg, output, outlen);
akcipher_request_set_crypt(req, &sig_sg, &digest_sg, sig->s_size,
outlen);
- init_completion(&compl.completion);
+ crypto_init_wait(&cwait);
akcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
CRYPTO_TFM_REQ_MAY_SLEEP,
- public_key_verify_done, &compl);
+ crypto_req_done, &cwait);
/* Perform the verification calculation. This doesn't actually do the
* verification, but rather calculates the hash expected by the
* signature and returns that to us.
*/
- ret = crypto_akcipher_verify(req);
- if ((ret == -EINPROGRESS) || (ret == -EBUSY)) {
- wait_for_completion(&compl.completion);
- ret = compl.err;
- }
+ ret = crypto_wait_req(crypto_akcipher_verify(req), &cwait);
if (ret < 0)
goto out_free_output;
--
2.1.4
^ permalink raw reply related
* [PATCH v8 14/20] cifs: move to generic async completion
From: Gilad Ben-Yossef @ 2017-09-05 12:38 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Tom Lendacky, Gary Hook, Boris Brezillon, Arnaud Ebalard,
Matthias Brugger, Alasdair Kergon, Mike Snitzer, dm-devel,
Shaohua Li, Steve French, Theodore Y. Ts'o, Jaegeuk Kim,
Steffen Klassert, Alexey Kuznetsov, Hideaki YOSHIFUJI, Mimi Zohar,
Dmitry Kasatkin, James Morris, Serge E. Hallyn, linux-crypto,
linux-doc, linux-kernel, keyrings, linux-arm-kernel,
linux-mediatek, linux-raid, linux-cifs, samba-technical,
linux-fscrypt, netdev, linux-ima-devel, linux-ima-user,
linux-security-module
Cc: Ofir Drang
In-Reply-To: <1504615144-29770-1-git-send-email-gilad@benyossef.com>
cifs starts an async. crypto op and waits for their completion.
Move it over to generic code doing the same.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
Acked-by: Pavel Shilovsky <pshilov@microsoft.com>
---
fs/cifs/smb2ops.c | 30 ++++--------------------------
1 file changed, 4 insertions(+), 26 deletions(-)
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index fb2934b..982b39d 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -2066,22 +2066,6 @@ init_sg(struct smb_rqst *rqst, u8 *sign)
return sg;
}
-struct cifs_crypt_result {
- int err;
- struct completion completion;
-};
-
-static void cifs_crypt_complete(struct crypto_async_request *req, int err)
-{
- struct cifs_crypt_result *res = req->data;
-
- if (err == -EINPROGRESS)
- return;
-
- res->err = err;
- complete(&res->completion);
-}
-
static int
smb2_get_enc_key(struct TCP_Server_Info *server, __u64 ses_id, int enc, u8 *key)
{
@@ -2122,12 +2106,10 @@ crypt_message(struct TCP_Server_Info *server, struct smb_rqst *rqst, int enc)
struct aead_request *req;
char *iv;
unsigned int iv_len;
- struct cifs_crypt_result result = {0, };
+ DECLARE_CRYPTO_WAIT(wait);
struct crypto_aead *tfm;
unsigned int crypt_len = le32_to_cpu(tr_hdr->OriginalMessageSize);
- init_completion(&result.completion);
-
rc = smb2_get_enc_key(server, tr_hdr->SessionId, enc, key);
if (rc) {
cifs_dbg(VFS, "%s: Could not get %scryption key\n", __func__,
@@ -2187,14 +2169,10 @@ crypt_message(struct TCP_Server_Info *server, struct smb_rqst *rqst, int enc)
aead_request_set_ad(req, assoc_data_len);
aead_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- cifs_crypt_complete, &result);
+ crypto_req_done, &wait);
- rc = enc ? crypto_aead_encrypt(req) : crypto_aead_decrypt(req);
-
- if (rc == -EINPROGRESS || rc == -EBUSY) {
- wait_for_completion(&result.completion);
- rc = result.err;
- }
+ rc = crypto_wait_req(enc ? crypto_aead_encrypt(req)
+ : crypto_aead_decrypt(req), &wait);
if (!rc && enc)
memcpy(&tr_hdr->Signature, sign, SMB2_SIGNATURE_SIZE);
--
2.1.4
^ permalink raw reply related
* [PATCH v8 15/20] ima: move to generic async completion
From: Gilad Ben-Yossef @ 2017-09-05 12:38 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Tom Lendacky, Gary Hook, Boris Brezillon, Arnaud Ebalard,
Matthias Brugger, Alasdair Kergon, Mike Snitzer, dm-devel,
Shaohua Li, Steve French, Theodore Y. Ts'o, Jaegeuk Kim,
Steffen Klassert, Alexey Kuznetsov, Hideaki YOSHIFUJI, Mimi Zohar,
Dmitry Kasatkin, James Morris, Serge E. Hallyn, linux-crypto,
linux-doc, linux-kernel, keyrings, linux-arm-kernel,
linux-mediatek, linux-raid, linux-cifs, samba-technical,
linux-fscrypt, netdev, linux-ima-devel, linux-ima-user,
linux-security-module
Cc: Ofir Drang
In-Reply-To: <1504615144-29770-1-git-send-email-gilad@benyossef.com>
ima starts several async crypto ops and waits for their completions.
Move it over to generic code doing the same.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
security/integrity/ima/ima_crypto.c | 56 +++++++++++--------------------------
1 file changed, 17 insertions(+), 39 deletions(-)
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index a856d8c..9057b16 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -27,11 +27,6 @@
#include "ima.h"
-struct ahash_completion {
- struct completion completion;
- int err;
-};
-
/* minimum file size for ahash use */
static unsigned long ima_ahash_minsize;
module_param_named(ahash_minsize, ima_ahash_minsize, ulong, 0644);
@@ -196,30 +191,13 @@ static void ima_free_atfm(struct crypto_ahash *tfm)
crypto_free_ahash(tfm);
}
-static void ahash_complete(struct crypto_async_request *req, int err)
+static inline int ahash_wait(int err, struct crypto_wait *wait)
{
- struct ahash_completion *res = req->data;
- if (err == -EINPROGRESS)
- return;
- res->err = err;
- complete(&res->completion);
-}
+ err = crypto_wait_req(err, wait);
-static int ahash_wait(int err, struct ahash_completion *res)
-{
- switch (err) {
- case 0:
- break;
- case -EINPROGRESS:
- case -EBUSY:
- wait_for_completion(&res->completion);
- reinit_completion(&res->completion);
- err = res->err;
- /* fall through */
- default:
+ if (err)
pr_crit_ratelimited("ahash calculation failed: err: %d\n", err);
- }
return err;
}
@@ -233,7 +211,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
int rc, read = 0, rbuf_len, active = 0, ahash_rc = 0;
struct ahash_request *req;
struct scatterlist sg[1];
- struct ahash_completion res;
+ struct crypto_wait wait;
size_t rbuf_size[2];
hash->length = crypto_ahash_digestsize(tfm);
@@ -242,12 +220,12 @@ static int ima_calc_file_hash_atfm(struct file *file,
if (!req)
return -ENOMEM;
- init_completion(&res.completion);
+ crypto_init_wait(&wait);
ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
CRYPTO_TFM_REQ_MAY_SLEEP,
- ahash_complete, &res);
+ crypto_req_done, &wait);
- rc = ahash_wait(crypto_ahash_init(req), &res);
+ rc = ahash_wait(crypto_ahash_init(req), &wait);
if (rc)
goto out1;
@@ -288,7 +266,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
* read/request, wait for the completion of the
* previous ahash_update() request.
*/
- rc = ahash_wait(ahash_rc, &res);
+ rc = ahash_wait(ahash_rc, &wait);
if (rc)
goto out3;
}
@@ -304,7 +282,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
* read/request, wait for the completion of the
* previous ahash_update() request.
*/
- rc = ahash_wait(ahash_rc, &res);
+ rc = ahash_wait(ahash_rc, &wait);
if (rc)
goto out3;
}
@@ -318,7 +296,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
active = !active; /* swap buffers, if we use two */
}
/* wait for the last update request to complete */
- rc = ahash_wait(ahash_rc, &res);
+ rc = ahash_wait(ahash_rc, &wait);
out3:
if (read)
file->f_mode &= ~FMODE_READ;
@@ -327,7 +305,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
out2:
if (!rc) {
ahash_request_set_crypt(req, NULL, hash->digest, 0);
- rc = ahash_wait(crypto_ahash_final(req), &res);
+ rc = ahash_wait(crypto_ahash_final(req), &wait);
}
out1:
ahash_request_free(req);
@@ -537,7 +515,7 @@ static int calc_buffer_ahash_atfm(const void *buf, loff_t len,
{
struct ahash_request *req;
struct scatterlist sg;
- struct ahash_completion res;
+ struct crypto_wait wait;
int rc, ahash_rc = 0;
hash->length = crypto_ahash_digestsize(tfm);
@@ -546,12 +524,12 @@ static int calc_buffer_ahash_atfm(const void *buf, loff_t len,
if (!req)
return -ENOMEM;
- init_completion(&res.completion);
+ crypto_init_wait(&wait);
ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
CRYPTO_TFM_REQ_MAY_SLEEP,
- ahash_complete, &res);
+ crypto_req_done, &wait);
- rc = ahash_wait(crypto_ahash_init(req), &res);
+ rc = ahash_wait(crypto_ahash_init(req), &wait);
if (rc)
goto out;
@@ -561,10 +539,10 @@ static int calc_buffer_ahash_atfm(const void *buf, loff_t len,
ahash_rc = crypto_ahash_update(req);
/* wait for the update request to complete */
- rc = ahash_wait(ahash_rc, &res);
+ rc = ahash_wait(ahash_rc, &wait);
if (!rc) {
ahash_request_set_crypt(req, NULL, hash->digest, 0);
- rc = ahash_wait(crypto_ahash_final(req), &res);
+ rc = ahash_wait(crypto_ahash_final(req), &wait);
}
out:
ahash_request_free(req);
--
2.1.4
^ permalink raw reply related
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