* Re: [PATCH v3 net-next] Introduce a sysctl that modifies the value of PROT_SOCK.
From: David Miller @ 2017-01-24 17:11 UTC (permalink / raw)
To: kjlx; +Cc: eric.dumazet, stephen, netdev
In-Reply-To: <20170121014911.GB2598@templeofstupid.com>
From: Krister Johansen <kjlx@templeofstupid.com>
Date: Fri, 20 Jan 2017 17:49:11 -0800
> Add net.ipv4.ip_unprivileged_port_start, which is a per namespace sysctl
> that denotes the first unprivileged inet port in the namespace. To
> disable all privileged ports set this to zero. It also checks for
> overlap with the local port range. The privileged and local range may
> not overlap.
>
> The use case for this change is to allow containerized processes to bind
> to priviliged ports, but prevent them from ever being allowed to modify
> their container's network configuration. The latter is accomplished by
> ensuring that the network namespace is not a child of the user
> namespace. This modification was needed to allow the container manager
> to disable a namespace's priviliged port restrictions without exposing
> control of the network namespace to processes in the user namespace.
>
> Signed-off-by: Krister Johansen <kjlx@templeofstupid.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH v5 0/2] Add support for the ethernet switch on the ESPRESSObin
From: Gregory CLEMENT @ 2017-01-24 16:47 UTC (permalink / raw)
To: David Miller, vivien.didelot
Cc: andrew, f.fainelli, netdev, linux-kernel, jason,
sebastian.hesselbarth, thomas.petazzoni, linux-arm-kernel, nadavh,
dingwei, kostap, shjzhou, jpannell
In-Reply-To: <20170120.141705.1390880809796992457.davem@davemloft.net>
Hi David,
On ven., janv. 20 2017, David Miller <davem@davemloft.net> wrote:
> From: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Date: Thu, 19 Jan 2017 22:49:32 +0100
>
>> I created a new family for this switch and filled the ops structure
>> by selecting which seems the more appropriate functions. I rebased
>> the series on net-next/master which allowed me to benefit to the
>> eeprom functions introduced for the 6390.
>
> It looks like there will be at least one more respin of this series,
> specifically to remove the new family as Vivien seems to object to
> this.
I am about to send a new version. However about removing the new family,
I thought that with the confirmation from Andrew we could keep it.
Vivien could you confirm this?
Thanks,
Gregory
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v2 00/18] gtp: fixes and support multiple VRF's per GTP socket
From: David Miller @ 2017-01-24 16:46 UTC (permalink / raw)
To: laforge; +Cc: aschultz, pablo, netdev, Lionel.Gauthier, openbsc
In-Reply-To: <20170124153919.zkigaq55kykyrfq5@nataraja>
From: Harald Welte <laforge@netfilter.org>
Date: Tue, 24 Jan 2017 16:39:19 +0100
> Hi Andreas,
>
> On Tue, Jan 24, 2017 at 04:28:30PM +0100, Andreas Schultz wrote:
>> During that work some smaller problems where found and fixes for them are
>> included.
>
> As this is a rather large patch-set, and I think it might be better to
> split it in clear bugfixes of the existing code (or no-brainers like the
> module alias) and in introducing new featur regarding socket/netdev.
> However, that's just my comment "from the side", I'm not the one who
> needs to be convinced in terms of merging them :)
>
> I would just assume that the clear fixes + no-brainers could go in right
> away without any further review...
Yes, please split this stuff up, the patch series is _way_ too large.
Anything more than a dozen patches is way too much to burdon reviewers
with at one time.
^ permalink raw reply
* Re: [for-next 07/10] IB/mlx5: Use blue flame register allocator in mlx5_ib
From: Doug Ledford @ 2017-01-24 16:44 UTC (permalink / raw)
To: Leon Romanovsky, David Miller
Cc: eli-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb,
saeedm-VPRAkNaXOzVWk0Htik3J/w, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, eli-VPRAkNaXOzVWk0Htik3J/w,
matanb-VPRAkNaXOzVWk0Htik3J/w
In-Reply-To: <20170108062257.GH15685-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1690 bytes --]
On Sun, 2017-01-08 at 08:22 +0200, Leon Romanovsky wrote:
> On Fri, Jan 06, 2017 at 11:11:31AM -0500, David Miller wrote:
> >
> > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Date: Fri, 6 Jan 2017 08:06:09 +0200
> >
> > >
> > > On Thu, Jan 05, 2017 at 03:07:31PM -0500, David Miller wrote:
> > > >
> > > > From: Eli Cohen <eli-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
> > > > Date: Thu, 5 Jan 2017 14:03:18 -0600
> > > >
> > > > >
> > > > > If necessary I can make sure it builds on 32 bits as well.
> > > >
> > > > Please do.
> > >
> > > Dave,
> > >
> > > I'm failing to understand the benefits of building mlx5 on 32
> > > bits, and
> > > see only disadvantages:
> > > * It is actual dead code without test coverage.
> > > * It misleads reviewers/customers by seeing code for 32 bits.
> > > * It adds compilation time for 32 bits platforms and "punishes"
> > > them
> > > for not relevant for them driver.
> > >
> > > Why do you call removing all that as a "regression"?
> >
> > We have this thing called "CONFIG_COMPILE_TEST", it has tons of
> > value,
> > perhaps you've seen it before?
>
> Thanks David,
> I see your point.
What's the status update on this. Unless I'm missing it in my rdma
mailing list (which is always possible if I scan subjects too fast), I
don't see a v2 of this pull request, but I also don't see that David's
concerns have been addressed, nor that he has pulled this.
--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: pull-request: mac80211 2017-01-24
From: David Miller @ 2017-01-24 16:42 UTC (permalink / raw)
To: johannes-cdvu00un1VgdHxzADdlk8Q
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170124153453.15776-1-johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
From: Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
Date: Tue, 24 Jan 2017 16:34:52 +0100
> LTP found a problem last week, but I couldn't fix it since I
> was travelling, so here's the fix now.
>
> Please pull and let me know if there's any problem.
Pulled, thanks Johannes.
^ permalink raw reply
* RE: [RFC PATCH net-next 2/5] net: split skb_checksum_help
From: David Laight @ 2017-01-24 16:35 UTC (permalink / raw)
To: 'Tom Herbert', Davide Caratti
Cc: David S. Miller, Linux Kernel Network Developers,
linux-sctp@vger.kernel.org, Marcelo Ricardo Leitner
In-Reply-To: <CALx6S37E=OEkRego_u=oDuUgQUn9-NZLpupvbN_dge1rqF92CQ@mail.gmail.com>
From: Tom Herbert
> Sent: 23 January 2017 21:00
..
> skb_checksum_help is specific to the Internet checksum. For instance,
> CHECKSUM_COMPLETE can _only_ refer to Internet checksum calculation
> nothing else will work. Checksums and CRCs are very different things
> with very different processing. They are not interchangeable, have
> very different properties, and hence it is a mistake to try to shoe
> horn things so that they use a common infrastructure.
>
> It might make sense to create some CRC helper functions, but last time
> I checked there are so few users of CRC in skbufs I'm not even sure
> that would make sense.
I can imagine horrid things happening if someone tries to encapsulate
SCTP/IP in UDP (or worse UDP/IP in SCTP).
For UDP in UDP I suspect that CHECKSUM_COMPLETE on an inner UDP packet
allows the outer checksum be calculated by ignoring the inner packet
(since it sums to zero).
This just isn't true if SCTP is involved.
There are tricks to generate a crc of a longer packet, but they'd only
work for SCTP in SCTP.
For non-encapsulated packets it is a different matter.
David
^ permalink raw reply
* Re: [PATCH v2 00/18] gtp: fixes and support multiple VRF's per GTP socket
From: Harald Welte @ 2017-01-24 15:39 UTC (permalink / raw)
To: Andreas Schultz; +Cc: Pablo Neira, netdev, Lionel Gauthier, openbsc
In-Reply-To: <20170124152848.6120-1-aschultz@tpip.net>
Hi Andreas,
On Tue, Jan 24, 2017 at 04:28:30PM +0100, Andreas Schultz wrote:
> During that work some smaller problems where found and fixes for them are
> included.
As this is a rather large patch-set, and I think it might be better to
split it in clear bugfixes of the existing code (or no-brainers like the
module alias) and in introducing new featur regarding socket/netdev.
However, that's just my comment "from the side", I'm not the one who
needs to be convinced in terms of merging them :)
I would just assume that the clear fixes + no-brainers could go in right
away without any further review...
--
- Harald Welte <laforge@netfilter.org> http://netfilter.org/
============================================================================
"Fragmentation is like classful addressing -- an interesting early
architectural error that shows how much experimentation was going
on while IP was being designed." -- Paul Vixie
^ permalink raw reply
* Re: [PATCH][V3][net-next] net: sctp: fix array overrun read on sctp_timer_tbl
From: Xin Long @ 2017-01-24 16:34 UTC (permalink / raw)
To: Colin King
Cc: Vlad Yasevich, Neil Horman, David S . Miller, linux-sctp,
network dev, LKML
In-Reply-To: <20170124092554.27019-1-colin.king@canonical.com>
On Tue, Jan 24, 2017 at 5:25 PM, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Table sctp_timer_tbl is missing a TIMEOUT_RECONF string so
> add this in. Also compare timeout with the size of the array
> sctp_timer_tbl rather than SCTP_EVENT_TIMEOUT_MAX. Also add
> a build time check that SCTP_EVENT_TIMEOUT_MAX is correct
> so we don't ever get this kind of mismatch between the table
> and SCTP_EVENT_TIMEOUT_MAX in the future.
>
> Kudos to Marcelo Ricardo Leitner for spotting the missing string
> and suggesting the build time sanity check.
>
> Fixes CoverityScan CID#1397639 ("Out-of-bounds read")
>
> Fixes: 7b9438de0cd4 ("sctp: add stream reconf timer")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Reviewed-by: Xin Long <lucien.xin@gmail.com>
^ permalink raw reply
* [PATCH net v3 1/2] net: Specify the owning module for lwtunnel ops
From: Robert Shearman @ 2017-01-24 16:26 UTC (permalink / raw)
To: davem; +Cc: netdev, Tom Herbert, Roopa Prabhu, Robert Shearman
In-Reply-To: <1485275208-31702-1-git-send-email-rshearma@brocade.com>
Modules implementing lwtunnel ops should not be allowed to unload
while there is state alive using those ops, so specify the owning
module for all lwtunnel ops.
Signed-off-by: Robert Shearman <rshearma@brocade.com>
---
include/net/lwtunnel.h | 2 ++
net/core/lwt_bpf.c | 1 +
net/ipv4/ip_tunnel_core.c | 2 ++
net/ipv6/ila/ila_lwt.c | 1 +
net/ipv6/seg6_iptunnel.c | 1 +
net/mpls/mpls_iptunnel.c | 1 +
6 files changed, 8 insertions(+)
diff --git a/include/net/lwtunnel.h b/include/net/lwtunnel.h
index 0b585f1fd340..73dd87647460 100644
--- a/include/net/lwtunnel.h
+++ b/include/net/lwtunnel.h
@@ -44,6 +44,8 @@ struct lwtunnel_encap_ops {
int (*get_encap_size)(struct lwtunnel_state *lwtstate);
int (*cmp_encap)(struct lwtunnel_state *a, struct lwtunnel_state *b);
int (*xmit)(struct sk_buff *skb);
+
+ struct module *owner;
};
#ifdef CONFIG_LWTUNNEL
diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
index 71bb3e2eca08..b3eef90b2df9 100644
--- a/net/core/lwt_bpf.c
+++ b/net/core/lwt_bpf.c
@@ -386,6 +386,7 @@ static const struct lwtunnel_encap_ops bpf_encap_ops = {
.fill_encap = bpf_fill_encap_info,
.get_encap_size = bpf_encap_nlsize,
.cmp_encap = bpf_encap_cmp,
+ .owner = THIS_MODULE,
};
static int __init bpf_lwt_init(void)
diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
index fed3d29f9eb3..0fd1976ab63b 100644
--- a/net/ipv4/ip_tunnel_core.c
+++ b/net/ipv4/ip_tunnel_core.c
@@ -313,6 +313,7 @@ static const struct lwtunnel_encap_ops ip_tun_lwt_ops = {
.fill_encap = ip_tun_fill_encap_info,
.get_encap_size = ip_tun_encap_nlsize,
.cmp_encap = ip_tun_cmp_encap,
+ .owner = THIS_MODULE,
};
static const struct nla_policy ip6_tun_policy[LWTUNNEL_IP6_MAX + 1] = {
@@ -403,6 +404,7 @@ static const struct lwtunnel_encap_ops ip6_tun_lwt_ops = {
.fill_encap = ip6_tun_fill_encap_info,
.get_encap_size = ip6_tun_encap_nlsize,
.cmp_encap = ip_tun_cmp_encap,
+ .owner = THIS_MODULE,
};
void __init ip_tunnel_core_init(void)
diff --git a/net/ipv6/ila/ila_lwt.c b/net/ipv6/ila/ila_lwt.c
index a7bc54ab46e2..13b5e85fe0d5 100644
--- a/net/ipv6/ila/ila_lwt.c
+++ b/net/ipv6/ila/ila_lwt.c
@@ -238,6 +238,7 @@ static const struct lwtunnel_encap_ops ila_encap_ops = {
.fill_encap = ila_fill_encap_info,
.get_encap_size = ila_encap_nlsize,
.cmp_encap = ila_encap_cmp,
+ .owner = THIS_MODULE,
};
int ila_lwt_init(void)
diff --git a/net/ipv6/seg6_iptunnel.c b/net/ipv6/seg6_iptunnel.c
index 1d60cb132835..c46f8cbf5ab5 100644
--- a/net/ipv6/seg6_iptunnel.c
+++ b/net/ipv6/seg6_iptunnel.c
@@ -422,6 +422,7 @@ static const struct lwtunnel_encap_ops seg6_iptun_ops = {
.fill_encap = seg6_fill_encap_info,
.get_encap_size = seg6_encap_nlsize,
.cmp_encap = seg6_encap_cmp,
+ .owner = THIS_MODULE,
};
int __init seg6_iptunnel_init(void)
diff --git a/net/mpls/mpls_iptunnel.c b/net/mpls/mpls_iptunnel.c
index 2f7ccd934416..1d281c1ff7c1 100644
--- a/net/mpls/mpls_iptunnel.c
+++ b/net/mpls/mpls_iptunnel.c
@@ -215,6 +215,7 @@ static const struct lwtunnel_encap_ops mpls_iptun_ops = {
.fill_encap = mpls_fill_encap_info,
.get_encap_size = mpls_encap_nlsize,
.cmp_encap = mpls_encap_cmp,
+ .owner = THIS_MODULE,
};
static int __init mpls_iptunnel_init(void)
--
2.1.4
^ permalink raw reply related
* [PATCH net v3 0/2] net: Fix oops on state free after lwt module unload
From: Robert Shearman @ 2017-01-24 16:26 UTC (permalink / raw)
To: davem; +Cc: netdev, Tom Herbert, Roopa Prabhu, Robert Shearman
In-Reply-To: <1484958086-6151-1-git-send-email-rshearma@brocade.com>
An oops is seen in lwtstate_free after an lwt ops module has been
unloaded. This patchset fixes this by preventing modules implementing
lwtunnel ops from being unloaded whilst there's state alive using
those ops. The first patch adds fills in a new owner field in all lwt
ops and the second patch makes use of this to reference count the
modules as state is built and destroyed using them.
Changes in v3:
- don't put module reference if try_module_get fails on building state
Changes in v2:
- specify module owner for all modules as suggested by DaveM
- reference count all modules building lwt state, not just those ops
implementing destroy_state, as also suggested by DaveM.
- rebased on top of David Ahern's lwtunnel changes
Robert Shearman (2):
net: Specify the owning module for lwtunnel ops
lwtunnel: Fix oops on state free after encap module unload
include/net/lwtunnel.h | 2 ++
net/core/lwt_bpf.c | 1 +
net/core/lwtunnel.c | 6 +++++-
net/ipv4/ip_tunnel_core.c | 2 ++
net/ipv6/ila/ila_lwt.c | 1 +
net/ipv6/seg6_iptunnel.c | 1 +
net/mpls/mpls_iptunnel.c | 1 +
7 files changed, 13 insertions(+), 1 deletion(-)
--
2.1.4
^ permalink raw reply
* [PATCH net v3 2/2] lwtunnel: Fix oops on state free after encap module unload
From: Robert Shearman @ 2017-01-24 16:26 UTC (permalink / raw)
To: davem; +Cc: netdev, Tom Herbert, Roopa Prabhu, Robert Shearman
In-Reply-To: <1485275208-31702-1-git-send-email-rshearma@brocade.com>
When attempting to free lwtunnel state after the module for the encap
has been unloaded an oops occurs:
BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
IP: lwtstate_free+0x18/0x40
[..]
task: ffff88003e372380 task.stack: ffffc900001fc000
RIP: 0010:lwtstate_free+0x18/0x40
RSP: 0018:ffff88003fd83e88 EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff88002bbb3380 RCX: ffff88000c91a300
[..]
Call Trace:
<IRQ>
free_fib_info_rcu+0x195/0x1a0
? rt_fibinfo_free+0x50/0x50
rcu_process_callbacks+0x2d3/0x850
? rcu_process_callbacks+0x296/0x850
__do_softirq+0xe4/0x4cb
irq_exit+0xb0/0xc0
smp_apic_timer_interrupt+0x3d/0x50
apic_timer_interrupt+0x93/0xa0
[..]
Code: e8 6e c6 fc ff 89 d8 5b 5d c3 bb de ff ff ff eb f4 66 90 66 66 66 66 90 55 48 89 e5 53 0f b7 07 48 89 fb 48 8b 04 c5 00 81 d5 81 <48> 8b 40 08 48 85 c0 74 13 ff d0 48 8d 7b 20 be 20 00 00 00 e8
The problem is after the module for the encap can be unloaded the
corresponding ops is removed and is thus NULL here.
Modules implementing lwtunnel ops should not be allowed to unload
while there is state alive using those ops, so grab the module
reference for the ops on creating lwtunnel state and of course release
the reference when freeing the state.
Fixes: 1104d9ba443a ("lwtunnel: Add destroy state operation")
Signed-off-by: Robert Shearman <rshearma@brocade.com>
---
net/core/lwtunnel.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/net/core/lwtunnel.c b/net/core/lwtunnel.c
index 47b1dd65947b..c23465005f2f 100644
--- a/net/core/lwtunnel.c
+++ b/net/core/lwtunnel.c
@@ -115,8 +115,11 @@ int lwtunnel_build_state(struct net_device *dev, u16 encap_type,
ret = -EOPNOTSUPP;
rcu_read_lock();
ops = rcu_dereference(lwtun_encaps[encap_type]);
- if (likely(ops && ops->build_state))
+ if (likely(ops && ops->build_state && try_module_get(ops->owner))) {
ret = ops->build_state(dev, encap, family, cfg, lws);
+ if (ret)
+ module_put(ops->owner);
+ }
rcu_read_unlock();
return ret;
@@ -194,6 +197,7 @@ void lwtstate_free(struct lwtunnel_state *lws)
} else {
kfree(lws);
}
+ module_put(ops->owner);
}
EXPORT_SYMBOL(lwtstate_free);
--
2.1.4
^ permalink raw reply related
* Re: [PATCH net v2 2/2] lwtunnel: Fix oops on state free after encap module unload
From: Robert Shearman @ 2017-01-24 16:26 UTC (permalink / raw)
To: David Miller; +Cc: netdev, tom, roopa, david.lebrun, tgraf, dsa
In-Reply-To: <20170123.154248.261820530578170635.davem@davemloft.net>
On 23/01/17 20:42, David Miller wrote:
> From: Robert Shearman <rshearma@brocade.com>
> Date: Sat, 21 Jan 2017 00:21:26 +0000
>
>> @@ -115,8 +115,12 @@ int lwtunnel_build_state(struct net_device *dev, u16 encap_type,
>> ret = -EOPNOTSUPP;
>> rcu_read_lock();
>
> Here 'ret' equals -EOPNOTSUPP
>
>> ops = rcu_dereference(lwtun_encaps[encap_type]);
>> - if (likely(ops && ops->build_state))
>> - ret = ops->build_state(dev, encap, family, cfg, lws);
>> + if (likely(ops)) {
>> + if (likely(try_module_get(ops->owner) && ops->build_state))
>> + ret = ops->build_state(dev, encap, family, cfg, lws);
>> + if (ret)
>> + module_put(ops->owner);
>
> If try_module_get() fails, 'ret' will still be -EOPNOTSUPP and we will
> module_put() on a module we did not grab a reference to.
>
> I think you need to adjust the logic here. You only want to 'put' if
> try_module_get() succeeds and ->build_state() returns an error.
Indeed, good point. Will address in a v3 shortly.
Thanks,
Rob
^ permalink raw reply
* Re: [PATCH 3/3] powerpc: bpf: implement in-register swap for 64-bit endian operations
From: 'Naveen N. Rao' @ 2017-01-24 16:25 UTC (permalink / raw)
To: David Laight
Cc: Benjamin Herrenschmidt, netdev@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, davem@davemloft.net,
daniel@iogearbox.net, ast@fb.com, Madhavan Srinivasan,
Michael Ellerman
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB026CE48@AcuExch.aculab.com>
On 2017/01/24 04:13PM, David Laight wrote:
> From: 'Naveen N. Rao'
> > Sent: 23 January 2017 19:22
> > On 2017/01/15 09:00AM, Benjamin Herrenschmidt wrote:
> > > On Fri, 2017-01-13 at 23:22 +0530, 'Naveen N. Rao' wrote:
> > > > > That rather depends on whether the processor has a store to load forwarder
> > > > > that will satisfy the read from the store buffer.
> > > > > I don't know about ppc, but at least some x86 will do that.
> > > >
> > > > Interesting - good to know that.
> > > >
> > > > However, I don't think powerpc does that and in-register swap is likely
> > > > faster regardless. Note also that gcc prefers this form at higher
> > > > optimization levels.
> > >
> > > Of course powerpc has a load-store forwarder these days, however, I
> > > wouldn't be surprised if the in-register form was still faster on some
> > > implementations, but this needs to be tested.
> >
> > Thanks for clarifying! To test this, I wrote a simple (perhaps naive)
> > test that just issues a whole lot of endian swaps and in _that_ test, it
> > does look like the load-store forwarder is doing pretty well.
> ...
> > This is all in a POWER8 vm. On POWER7, the in-register variant is around
> > 4 times faster than the ldbrx variant.
> ...
>
> I wonder which is faster on the little 1GHz embedded ppc we use here.
Worth a test, for sure.
FWIW, this patch won't matter since eBPF JIT is for ppc64.
Thanks,
Naveen
^ permalink raw reply
* Re: [PATCH net] net: reset ct before calling ndo_start_xmit
From: David Miller @ 2017-01-24 16:21 UTC (permalink / raw)
To: pabeni; +Cc: netdev, hannes, fw
In-Reply-To: <e33476125bdaf4619c221aa82c034831bbdce89b.1485250657.git.pabeni@redhat.com>
From: Paolo Abeni <pabeni@redhat.com>
Date: Tue, 24 Jan 2017 10:40:13 +0100
> Some devices (e.g. ipoib and most wifi drivers) can retain the
> to-be-xmitted packets on some internal queue for a possibly
> unlimited time.
This is really not legal, such behavior also potentially holds onto
sockets forever.
If driver really wants to hold onto an SKB potentially forever, it
must release these resources.
This belongs in the driver that violates, and I strongly say
"violates", the basic fundamental rules of TX handling in the stack.
Drivers absolutely must liberate queued TX packets in a reasonable,
finite, amount of time.
^ permalink raw reply
* RE: [PATCH 3/3] powerpc: bpf: implement in-register swap for 64-bit endian operations
From: David Laight @ 2017-01-24 16:13 UTC (permalink / raw)
To: 'Naveen N. Rao', Benjamin Herrenschmidt
Cc: netdev@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
davem@davemloft.net, daniel@iogearbox.net, ast@fb.com,
Madhavan Srinivasan, Michael Ellerman
In-Reply-To: <20170123192227.GE3820@naverao1-tp.localdomain>
From: 'Naveen N. Rao'
> Sent: 23 January 2017 19:22
> On 2017/01/15 09:00AM, Benjamin Herrenschmidt wrote:
> > On Fri, 2017-01-13 at 23:22 +0530, 'Naveen N. Rao' wrote:
> > > > That rather depends on whether the processor has a store to load forwarder
> > > > that will satisfy the read from the store buffer.
> > > > I don't know about ppc, but at least some x86 will do that.
> > >
> > > Interesting - good to know that.
> > >
> > > However, I don't think powerpc does that and in-register swap is likely
> > > faster regardless. Note also that gcc prefers this form at higher
> > > optimization levels.
> >
> > Of course powerpc has a load-store forwarder these days, however, I
> > wouldn't be surprised if the in-register form was still faster on some
> > implementations, but this needs to be tested.
>
> Thanks for clarifying! To test this, I wrote a simple (perhaps naive)
> test that just issues a whole lot of endian swaps and in _that_ test, it
> does look like the load-store forwarder is doing pretty well.
...
> This is all in a POWER8 vm. On POWER7, the in-register variant is around
> 4 times faster than the ldbrx variant.
...
I wonder which is faster on the little 1GHz embedded ppc we use here.
David
^ permalink raw reply
* Re: [PATCHv2 perf/core 0/7] Libbpf improvements
From: Arnaldo Carvalho de Melo @ 2017-01-24 16:12 UTC (permalink / raw)
To: Joe Stringer; +Cc: wangnan0, ast, daniel, linux-kernel, netdev
In-Reply-To: <20170123011128.26534-1-joe@ovn.org>
Em Sun, Jan 22, 2017 at 05:11:21PM -0800, Joe Stringer escreveu:
> Patch 1 fixes an issue when using drastically different BPF map definitions
> inside ELFs from a client using libbpf, vs the map definition libbpf uses.
>
> Patches 2-4 add some simple, useful helper functions for setting prog type
> and retrieving libbpf errors without depending on kernel headers from
> userspace programs.
>
> Patches 5-7 add a new pinning functionality for maps, programs, and objects.
> Library users may call bpf_map__pin(map, path) or bpf_program__pin(prog, path)
> to pin maps and programs separately, or use bpf_object__pin(obj, path) to
> pin all maps and programs from the BPF object to the path. The map and program
> variations require a full path where it will be pinned in the filesystem,
> and the object variation will create directories "maps/" and "progs/" under
> the specified path, then mount each map and program under those subdirectories.
Merged the ones either acked by Wang or adjusted by you to address
Wang's remarks, the last ones introducing those __pin() methods, please
provide users together with those APIs, preferably entries for 'perf
test',
- Arnaldo
> ---
> v1: Initial post.
> v2: Wang Nan provided improvements to patch 1.
> Dropped patch 2 from v1.
> Added acks for acked patches.
> Split the bpf_obj__pin() to also provide map / program pinning APIs.
> Allow users to provide full filesystem path (don't autodetect/mount BPFFS).
>
> Joe Stringer (7):
> tools lib bpf: Fix map offsets in relocation
> tools lib bpf: Define prog_type fns with macro
> tools lib bpf: Add set/is helpers for all prog types
> tools lib bpf: Add libbpf_get_error()
> tools lib bpf: Add bpf_program__pin()
> tools lib bpf: Add bpf_map__pin()
> tools lib bpf: Add bpf_object__pin()
>
> tools/lib/bpf/libbpf.c | 240 ++++++++++++++++++++++++++++++++++++++++++------
> tools/lib/bpf/libbpf.h | 17 +++-
> tools/perf/tests/llvm.c | 2 +-
> 3 files changed, 229 insertions(+), 30 deletions(-)
>
> --
> 2.11.0
^ permalink raw reply
* [PATCH RFC net-next] packet: always ensure that we pass hard_header_len bytes in skb_headlen() to the driver
From: Sowmini Varadhan @ 2017-01-24 16:11 UTC (permalink / raw)
To: davem, sowmini.varadhan, sowmini.varadhan; +Cc: netdev
The contract between the socket layer and the device is that there
will be at least enough bytes in the non-paged part of the skb to
cover a link layer header, and this is ensured by copying any
application provided L2 header into the skb->data and then invoking
dev_validate_header().
If the application has provided fewer than hard_header_len bytes,
dev_validate_header() will zero out the skb->data as needed. This is
acceptable for SOCK_DGRAM/PF_PACKET sockets but in all other cases,
the application must provide a full L2 header, and the PF_PACKET Tx
paths must fail with an error when fewer than hard_header_len bytes
are detected.
All invocations to dev_validate_header() already adjusts the
skb's data, len, tail etc pointers based on hard_header_len before
invoking dev_validate_header(), so additional skb pointers should
not be needed after dev_validate_header().
Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
---
include/linux/netdevice.h | 11 ++++++-----
net/packet/af_packet.c | 27 +++++++++++++++++++++------
2 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 3868c32..9d49898 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2685,21 +2685,22 @@ static inline int dev_parse_header(const struct sk_buff *skb,
}
/* ll_header must have at least hard_header_len allocated */
-static inline bool dev_validate_header(const struct net_device *dev,
+static inline int dev_validate_header(const struct net_device *dev,
char *ll_header, int len)
{
if (likely(len >= dev->hard_header_len))
- return true;
+ return len;
if (capable(CAP_SYS_RAWIO)) {
memset(ll_header + len, 0, dev->hard_header_len - len);
- return true;
+ return dev->hard_header_len;
}
if (dev->header_ops && dev->header_ops->validate)
- return dev->header_ops->validate(ll_header, len);
+ if (!dev->header_ops->validate(ll_header, len))
+ return -1;
- return false;
+ return dev->hard_header_len;
}
typedef int gifconf_func_t(struct net_device * dev, char __user * bufptr, int len);
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index ddbda25..7af09a3 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1845,6 +1845,7 @@ static int packet_sendmsg_spkt(struct socket *sock, struct msghdr *msg,
__be16 proto = 0;
int err;
int extra_len = 0;
+ int newlen;
/*
* Get and verify the address.
@@ -1920,7 +1921,11 @@ static int packet_sendmsg_spkt(struct socket *sock, struct msghdr *msg,
goto retry;
}
- if (!dev_validate_header(dev, skb->data, len)) {
+ newlen = dev_validate_header(dev, skb->data, len);
+ /* As comments above this function indicate, a full L2 header
+ * must be passed to this function, so if newlen > len, bail.
+ */
+ if (newlen < 0 || newlen > len) {
err = -EINVAL;
goto out_unlock;
}
@@ -2447,14 +2452,21 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
return -EINVAL;
} else if (copylen) {
int hdrlen = min_t(int, copylen, tp_len);
+ int newlen;
skb_push(skb, dev->hard_header_len);
skb_put(skb, copylen - dev->hard_header_len);
err = skb_store_bits(skb, 0, data, hdrlen);
if (unlikely(err))
return err;
- if (!dev_validate_header(dev, skb->data, hdrlen))
+ newlen = dev_validate_header(dev, skb->data, hdrlen);
+ if (newlen < 0)
return -EINVAL;
+ /* Caller has allocated for copylen in non-paged part of
+ * skb so we should never find newlen > hdrlen
+ */
+ WARN_ON(newlen > hdrlen);
+
if (!skb->protocol)
tpacket_set_protocol(dev, skb);
@@ -2857,10 +2869,13 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
if (err)
goto out_free;
- if (sock->type == SOCK_RAW &&
- !dev_validate_header(dev, skb->data, len)) {
- err = -EINVAL;
- goto out_free;
+ if (sock->type == SOCK_RAW) {
+ int newlen = dev_validate_header(dev, skb->data, len);
+
+ if (newlen < 0 || newlen > len) {
+ err = -EINVAL;
+ goto out_free;
+ }
}
sock_tx_timestamp(sk, sockc.tsflags, &skb_shinfo(skb)->tx_flags);
--
1.7.1
^ permalink raw reply related
* Re: [RFC] net: ipv6: return the first matched rt6_info for multicast packets in find_rr_leaf()
From: Rajasekar Kumar @ 2017-01-24 16:10 UTC (permalink / raw)
To: David Miller; +Cc: kuznet, jmorris, yoshfuji, kaber, netdev, linux-kernel
In-Reply-To: <20170120.115804.2161107773230365505.davem@davemloft.net>
On Fri, Jan 20, 2017 at 11:58:04AM -0500, David Miller wrote:
> From: Rajasekar Kumar <sekraj@gmail.com>
> Date: Wed, 18 Jan 2017 20:43:37 +0530
>
> > There is a performance issue when large number of interfaces are
> > enabled with VRRP protocol in 2 router nodes which are connected
> > to each other. When VRRP hello is received (which is multicast
> > packet with DIP: ff02::18), a rt6_info node is added to fib6_node
> > of address ff02::18. This happens for each interface on which
> > VRRP is enabled. For 2000 interfaces with VRRP enabled, 2000
> > rt6_info nodes are added to the same fib6_node. As of today,
> > find_rr_leaf() goes further to find better match, even after first
> > successful match based on interface key. In this case, it walks
> > 2000 nodes for every incoming packet/outgoing packet, which is
> > expensive and not needed. rt6_info match based on supplied
> > interface match should be sufficient. The first match occurs
> > when there is interface match, and after that there can not be
> > another match for multicast packets. So, first match should be
> > returned for multicast packets.
> >
> > find_rr_leaf() tries to find best available gateway, mainly based on
> > interface match and gateway's reachablity info.When this is required
> > for unicast packets, multicast packets do not need either gateway's
> > reachability status or gateway's Layer2 address as it is derived
> > from Destination IP (group address). rt6_info match based on supplied
> > interface match should be sufficient.
> >
> > This fix helps in scenario wherein multicast packets arrive in some
> > interfaces frequently than other interfaces. rt6_info is added to
> > beginning of list for former cases. Verified this case.
> >
> > Signed-off-by: Rajasekar Kumar <sekraj@gmail.com>
>
> So the only thing different in each rt6_info in the list is the
> interface, right?
>
> Well, that's a part of the lookup key, multicast or not. If the user
> binds a socket to a specific interface, they want the route lookup to
> return the rt6_info node with that device.
>
> So I think your change introduces a regression, therefore another
> solution will need to be found for your performance problem.
Thanks for the review. Below are my thoughts.
The meaningful difference between rt6_infos is interface,
which is true for multicast. For unicast routes, nexthop's
reachability status, route's preference values also are important.
For unicast destinations, there will not be so many rt6_info's
for the same prefix even with multipath support, unlike multicast
case described above. In general for unicast routes, finding better
route/nexthop match involves gateway's reachability status, route's
preference values, in addition to interface match. This code is
present already. Also, it does not look like, IPv4 implementation
does any specific check for bind-to-device case in the area of
route lookup, but it does interface match like anyother case by
walking all nexthops in fib_info within check_leaf(). So, I am
not sure, if a fix is needed right now for non-multicast.
Comparing with IPv4 implementation with respect to IP packet
reception/transmission, multicast was handled specially in multiple
occurrences. This fix attempted to do something similar. If any
specific problem identified for non-multicast, can be addressed in
future. Please post your thoughts.
Sorry, I didnot understand your comment regarding regression.
Is the fix already creating regression issue?
^ permalink raw reply
* Re: [PATCH net-next v2 0/2] net: ipv6: Improve user experience with multipath routes
From: David Miller @ 2017-01-24 16:06 UTC (permalink / raw)
To: dsa; +Cc: netdev
In-Reply-To: <90d6fdce-7780-b864-0e89-db262dec6fa1@cumulusnetworks.com>
From: David Ahern <dsa@cumulusnetworks.com>
Date: Fri, 20 Jan 2017 12:31:44 -0700
> BTW, I am in favor of not requiring a user API for this but just
> doing it. I can't imagine anyone working with multipath routes not
> wanting the efficiency of the RTA_MULTIPATH attribute. These patches
> require an API only because of the rule not to break userspace. If
> we conclude to just do it without an API, the multipath_add and
> multipath_del need to be modified to only send a notification once
> at the end of the actions.
After some consideration, I agree.
Please repost this series without the new netlink knob.
Thanks.
^ permalink raw reply
* Re: [patch] samples/bpf: silence shift wrapping warning
From: Arnaldo Carvalho de Melo @ 2017-01-24 16:02 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Arnaldo Carvalho de Melo, Dan Carpenter, Thomas Graf,
Alexei Starovoitov, Joe Stringer, David S. Miller,
linux-kernel@vger.kernel.org, kernel-janitors,
netdev@vger.kernel.org
In-Reply-To: <CAADnVQLF6PWEjic6aSRBnRqtO25eEw7YLE+Gb95+f5_pvq3ZMA@mail.gmail.com>
Em Mon, Jan 23, 2017 at 10:44:34PM -0800, Alexei Starovoitov escreveu:
> On Mon, Jan 23, 2017 at 5:27 AM, Arnaldo Carvalho de Melo
> <arnaldo.melo@gmail.com> wrote:
> > Em Sun, Jan 22, 2017 at 02:51:25PM -0800, Alexei Starovoitov escreveu:
> >> On Sat, Jan 21, 2017 at 07:51:43AM +0300, Dan Carpenter wrote:
> >> > max_key is a value in the 0-63 range, so on 32 bit systems the shift
> >> > could wrap.
> >> >
> >> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> >>
> >> Looks fine. I think 'net-next' is ok.
> >
> > I could process these patches, if that would help,
>
> Thanks for the offer.
> I don't think there will be conflicts with all the work happening in net-next,
> but it's best to avoid even possibility of it when we can.
Okay sir, I'll let you know when/if the tests I perform building
samples/bpf/ in my containers catch something,
- Arnaldo
> Dan,
> can you please resend the patch cc-ing Dave and netdev ?
> please mention [PATCH net-next] in the subject.
>
> > - Arnaldo
> >
> >> Acked-by: Alexei Starovoitov <ast@kernel.org>
> >
> >> > diff --git a/samples/bpf/lwt_len_hist_user.c b/samples/bpf/lwt_len_hist_user.c
> >> > index ec8f3bb..bd06eef 100644
> >> > --- a/samples/bpf/lwt_len_hist_user.c
> >> > +++ b/samples/bpf/lwt_len_hist_user.c
> >> > @@ -68,7 +68,7 @@ int main(int argc, char **argv)
> >> > for (i = 1; i <= max_key + 1; i++) {
> >> > stars(starstr, data[i - 1], max_value, MAX_STARS);
> >> > printf("%8ld -> %-8ld : %-8ld |%-*s|\n",
> >> > - (1l << i) >> 1, (1l << i) - 1, data[i - 1],
> >> > + (1ULL << i) >> 1, (1ULL << i) - 1, data[i - 1],
> >> > MAX_STARS, starstr);
> >> > }
> >> >
^ permalink raw reply
* Re: [PATCH net-next 1/5] net: dsa: mv88e6xxx: Abstract mv88e6165 PHY operations
From: Vivien Didelot @ 2017-01-24 15:56 UTC (permalink / raw)
To: Andrew Lunn, David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn
In-Reply-To: <1485266031-4980-2-git-send-email-andrew@lunn.ch>
Andrew Lunn <andrew@lunn.ch> writes:
> The mv88e6165 family has the internal PHYs mapped directly onto the
> SMI register space as the switch. So the registers can be read
> directly. Put a wrapper around this, in preparation for changing the
> signature in order to support the external MDIO bus of the 6390.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
^ permalink raw reply
* Re: [PATCH 3/3] sh_eth: stop using bare numbers for EESIPR values
From: Sergei Shtylyov @ 2017-01-24 15:54 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: netdev@vger.kernel.org, Linux-Renesas
In-Reply-To: <CAMuHMdWWFmDkJfhac9jLZ-ZAJfzDPw7oq4hwL5hVy21h0V13Og@mail.gmail.com>
On 01/23/2017 11:00 AM, Geert Uytterhoeven wrote:
>> Now that we have almost all EESIPR bits declared (and those that are
>> still not are most probably reserved anyway) we can at last replace the
>> bare numbers used for 'sh_eth_cpu_data::eesipr_value' initializers with
>> the bit names ORed together...
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>
>> ---
>> drivers/net/ethernet/renesas/sh_eth.c | 89 +++++++++++++++++++++++++++++-----
>> 1 file changed, 78 insertions(+), 11 deletions(-)
>>
>> Index: net-next/drivers/net/ethernet/renesas/sh_eth.c
>> ===================================================================
>> --- net-next.orig/drivers/net/ethernet/renesas/sh_eth.c
>> +++ net-next/drivers/net/ethernet/renesas/sh_eth.c
>
>> @@ -800,7 +843,12 @@ static struct sh_eth_cpu_data sh7734_dat
>>
>> .ecsr_value = ECSR_ICD | ECSR_MPD,
>> .ecsipr_value = ECSIPR_LCHNGIP | ECSIPR_ICDIP | ECSIPR_MPDIP,
>> - .eesipr_value = EESIPR_RFCOFIP | EESIPR_ECIIP | 0x003f07ff,
>> + .eesipr_value = EESIPR_RFCOFIP | EESIPR_ECIIP |
>> + EESIPR_FTCIP | EESIPR_TDEIP | EESIPR_TFUFIP |
>> + EESIPR_FRIP | EESIPR_RDEIP | EESIPR_RFOFIP |
>
> Missing:
>
> EESIPR_DLCIP | EESIPR_CDIP | EESIPR_TROIP |
Hm, I thought I'd double-checked all the initializers... TY!
>> + EESIPR_RMAFIP | EESIPR_CEEFIP | EESIPR_CELFIP |
>> + EESIPR_RRFIP | EESIPR_RTLFIP | EESIPR_RTSFIP |
>> + EESIPR_PREIP | EESIPR_CERFIP,
>>
>> .tx_check = EESR_TC1 | EESR_FTC,
>> .eesr_err_check = EESR_TWB1 | EESR_TWB | EESR_TABT | EESR_RABT |
>> @@ -830,7 +878,12 @@ static struct sh_eth_cpu_data sh7763_dat
>>
>> .ecsr_value = ECSR_ICD | ECSR_MPD,
>> .ecsipr_value = ECSIPR_LCHNGIP | ECSIPR_ICDIP | ECSIPR_MPDIP,
>> - .eesipr_value = EESIPR_RFCOFIP | EESIPR_ECIIP | 0x003f07ff,
>> + .eesipr_value = EESIPR_RFCOFIP | EESIPR_ECIIP |
>> + EESIPR_FTCIP | EESIPR_TDEIP | EESIPR_TFUFIP |
>> + EESIPR_FRIP | EESIPR_RDEIP | EESIPR_RFOFIP |
>
> Likewise
:-<
>> + EESIPR_RMAFIP | EESIPR_CEEFIP | EESIPR_CELFIP |
>> + EESIPR_RRFIP | EESIPR_RTLFIP | EESIPR_RTSFIP |
>> + EESIPR_PREIP | EESIPR_CERFIP,
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Will add, thanks again...
> Gr{oetje,eeting}s,
>
> Geert
MBR, Sergei
^ permalink raw reply
* Re: net: use-after-free in tw_timer_handler
From: Eric Dumazet @ 2017-01-24 15:52 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: Eric Dumazet, David Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, syzkaller
In-Reply-To: <CACT4Y+b=-XJCwofbCRo6BnKJRU=amd7pxYpNrGdPEFT0bAtJaQ@mail.gmail.com>
On Tue, Jan 24, 2017 at 7:06 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
>>
>> This code was changed a long time ago :
>>
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=ed2e923945892a8372ab70d2f61d364b0b6d9054
>>
>> So I suspect a recent patch broke the logic.
>>
>> You might start a bisection :
>>
>> I would check if 4.7 and 4.8 trigger the issue you noticed.
>
>
> It happens with too low rate for bisecting (few times per day). I
> could add some additional checks into code, but I don't know what
> checks could be useful.
If you can not tell if 4.7 and/or 4.8 have the problem, I am not sure
we are able to help.
^ permalink raw reply
* Re: [PATCH 2/3] sh_eth: add missing EESIPR bits
From: Sergei Shtylyov @ 2017-01-24 15:51 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: netdev@vger.kernel.org, Linux-Renesas
In-Reply-To: <CAMuHMdVED1D46zGkuSbpF22ZnKKjWcf50yRp-uT+LcBY_70xgA@mail.gmail.com>
On 01/23/2017 10:41 AM, Geert Uytterhoeven wrote:
>> Renesas SH77{34|63} manuals describe more EESIPR bits than the current
>> driver. Declare the new bits with the end goal of using the bit names
>> instead of the bare numbers for the 'sh_eth_cpu_data::eesipr_value'
>> initializers...
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
>> ---
>> drivers/net/ethernet/renesas/sh_eth.h | 10 ++++++++--
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> Index: net-next/drivers/net/ethernet/renesas/sh_eth.h
>> ===================================================================
>> --- net-next.orig/drivers/net/ethernet/renesas/sh_eth.h
>> +++ net-next/drivers/net/ethernet/renesas/sh_eth.h
>> @@ -269,13 +269,17 @@ enum EESR_BIT {
>>
>> /* EESIPR */
>> enum EESIPR_BIT {
>> - EESIPR_TWBIP = 0x40000000,
>> + EESIPR_TWB1IP = 0x80000000,
>> + EESIPR_TWBIP = 0x40000000, /* same as TWB0IP */
>
> Ah, your adding it here ;-)
Do you want me to add such comment in the 1st patch instead?
> Gr{oetje,eeting}s,
>
> Geert
MRB, Sergei
^ permalink raw reply
* Re: [RFC PATCH net-next 0/5] bridge: per vlan lwt and dst_metadata support
From: Stephen Hemminger @ 2017-01-24 15:47 UTC (permalink / raw)
To: Roopa Prabhu
Cc: netdev, davem, nikolay, tgraf, hannes, jbenc, pshelar, dsa, hadi
In-Reply-To: <1484977616-1541-1-git-send-email-roopa@cumulusnetworks.com>
On Fri, 20 Jan 2017 21:46:51 -0800
Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> High level summary:
> lwt and dst_metadata/collect_metadata have enabled vxlan l3 deployments
> to use a single vxlan netdev for multiple vnis eliminating the scalability
> problem with using a single vxlan netdev per vni. This series tries to
> do the same for vxlan netdevs in pure l2 bridged networks.
> Use-case/deployment and details are below.
>
> Deployment scerario details:
> As we know VXLAN is used to build layer 2 virtual networks across the
> underlay layer3 infrastructure. A VXLAN tunnel endpoint (VTEP)
> originates and terminates VXLAN tunnels. And a VTEP can be a TOR switch
> or a vswitch in the hypervisor. This patch series mainly
> focuses on the TOR switch configured as a Vtep. Vxlan segment ID (vni)
> along with vlan id is used to identify layer 2 segments in a vxlan
> overlay network. Vxlan bridging is the function provided by Vteps to terminate
> vxlan tunnels and map the vxlan vni to traditional end host vlan. This is
> covered in the "VXLAN Deployment Scenarios" in sections 6 and 6.1 in RFC 7348.
> To provide vxlan bridging function, a vtep has to map vlan to a vni. The rfc
> says that the ingress VTEP device shall remove the IEEE 802.1Q VLAN tag in
> the original Layer 2 packet if there is one before encapsulating the packet
> into the VXLAN format to transmit it through the underlay network. The remote
> VTEP devices have information about the VLAN in which the packet will be
> placed based on their own VLAN-to-VXLAN VNI mapping configurations.
>
> Existing solution:
> Without this patch series one can deploy such a vtep configuration by
> by adding the local ports and vxlan netdevs into a vlan filtering bridge.
> The local ports are configured as trunk ports carrying all vlans.
> A vxlan netdev per vni is added to the bridge. Vlan mapping to vni is
> achieved by configuring the vlan as pvid on the corresponding vxlan netdev.
> The vxlan netdev only receives traffic corresponding to the vlan it is mapped
> to. This configuration maps traffic belonging to a vlan to the corresponding
> vxlan segment.
>
> -----------------------------------
> | bridge |
> | |
> -----------------------------------
> |100,200 |100 (pvid) |200 (pvid)
> | | |
> swp1 vxlan1000 vxlan2000
>
> This provides the required vxlan bridging function but poses a
> scalability problem with using a single vxlan netdev for each vni.
>
> Solution in this patch series:
> The Goal is to use a single vxlan device to carry all vnis similar
> to the vxlan collect metadata mode but vxlan driver still carrying all
> the forwarding information.
> - vxlan driver changes:
> - enable collect metadata mode device to be used with learning,
> replication, fdb
> - A single fdb table hashed by (mac, vni)
> - rx path already has the vni
> - tx path expects a vni in the packet with dst_metadata and vxlan
> driver has all the forwarding information for the vni in the
> dst_metadata.
>
> - Bridge driver changes: per vlan LWT and dst_metadata support:
> - Our use case is vxlan and 1-1 mapping between vlan and vni, but I have
> kept the api generic for any tunnel info
> - Uapi to configure/unconfigure/dump per vlan tunnel data
> - new bridge port flag to turn this feature on/off. off by default
> - ingress hook:
> - if port is a lwt tunnel port, use tunnel info in
> attached dst_metadata to map it to a local vlan
> - egress hook:
> - if port is a lwt tunnel port, use tunnel info attached to vlan
> to set dst_metadata on the skb
>
> Other approaches tried and vetoed:
> - tc vlan push/pop and tunnel metadata dst:
> - posses a tc rule scalability problem (2 rules per vni)
> - cannot handle the case where a packet needs to be replicated to
> multiple vxlan remote tunnel end-points.. which the vxlan driver
> can do today by having multiple remote destinations per fdb.
> - making vxlan driver understand vlan-vni mapping:
> - I had a series almost ready with this one but soon realized
> it duplicated a lot of vlan handling code in the vxlan driver
>
> This series is briefly tested for functionality. Sending it out as RFC while
> I continue to test it more. There are some rough edges which I am in the process
> of fixing.
>
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> Roopa Prabhu (5):
> ip_tunnels: new IP_TUNNEL_INFO_BRIDGE flag for ip_tunnel_info mode
> vxlan: make COLLECT_METADATA mode bridge friendly
> bridge: uapi: add per vlan tunnel info
> bridge: vlan lwt and dst_metadata netlink support
> bridge: vlan lwt dst_metadata hooks in ingress and egress paths
>
> drivers/net/vxlan.c | 209 ++++++++++++--------
> include/linux/if_bridge.h | 1 +
> include/net/ip_tunnels.h | 1 +
> include/uapi/linux/if_bridge.h | 11 ++
> include/uapi/linux/if_link.h | 1 +
> include/uapi/linux/neighbour.h | 1 +
> net/bridge/br_input.c | 5 +
> net/bridge/br_netlink.c | 410 ++++++++++++++++++++++++++++++++++------
> net/bridge/br_private.h | 22 +++
> net/bridge/br_vlan.c | 193 ++++++++++++++++++-
> 10 files changed, 717 insertions(+), 137 deletions(-)
>
Yes this is a complex issue, but I am concerned about the added code bloat
and complexity. The bridge which is already a mess with netfilter, multicast, vlan
filtering etc.The current code not modular and grows with each feature.
At the same time, the same bridge functionality is used in its simplest form by
all the network virtualization and container technolgies.
Maybe do it in OpenVswitch or figure out a way to do customization with BPF?
more complex than it should
^ 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