* [PATCH v5 7/9] task_isolation: don't interrupt CPUs with tick_nohz_full_kick_cpu()
From: Alex Belits @ 2020-11-23 17:58 UTC (permalink / raw)
To: nitesh@redhat.com, frederic@kernel.org
Cc: Prasun Kapoor, linux-api@vger.kernel.org, davem@davemloft.net,
trix@redhat.com, mingo@kernel.org, catalin.marinas@arm.com,
rostedt@goodmis.org, linux-kernel@vger.kernel.org,
peterx@redhat.com, tglx@linutronix.de, linux-arch@vger.kernel.org,
mtosatti@redhat.com, will@kernel.org, peterz@infradead.org,
leon@sidebranch.com, linux-arm-kernel@lists.infradead.org,
pauld@redhat.com, netdev@vger.kernel.org
In-Reply-To: <8d887e59ca713726f4fcb25a316e1e932b02823e.camel@marvell.com>
From: Yuri Norov <ynorov@marvell.com>
For nohz_full CPUs the desirable behavior is to receive interrupts
generated by tick_nohz_full_kick_cpu(). But for hard isolation it's
obviously not desirable because it breaks isolation.
This patch adds check for it.
Signed-off-by: Yuri Norov <ynorov@marvell.com>
[abelits@marvell.com: updated, only exclude CPUs running isolated tasks]
Signed-off-by: Alex Belits <abelits@marvell.com>
---
kernel/time/tick-sched.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index a213952541db..6c8679e200f0 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -20,6 +20,7 @@
#include <linux/sched/clock.h>
#include <linux/sched/stat.h>
#include <linux/sched/nohz.h>
+#include <linux/isolation.h>
#include <linux/module.h>
#include <linux/irq_work.h>
#include <linux/posix-timers.h>
@@ -268,7 +269,8 @@ static void tick_nohz_full_kick(void)
*/
void tick_nohz_full_kick_cpu(int cpu)
{
- if (!tick_nohz_full_cpu(cpu))
+ smp_rmb();
+ if (!tick_nohz_full_cpu(cpu) || task_isolation_on_cpu(cpu))
return;
irq_work_queue_on(&per_cpu(nohz_full_kick_work, cpu), cpu);
--
2.20.1
^ permalink raw reply related
* [PATCH v5 9/9] task_isolation: kick_all_cpus_sync: don't kick isolated cpus
From: Alex Belits @ 2020-11-23 17:58 UTC (permalink / raw)
To: nitesh@redhat.com, frederic@kernel.org
Cc: Prasun Kapoor, linux-api@vger.kernel.org, davem@davemloft.net,
trix@redhat.com, mingo@kernel.org, catalin.marinas@arm.com,
rostedt@goodmis.org, linux-kernel@vger.kernel.org,
peterx@redhat.com, tglx@linutronix.de, linux-arch@vger.kernel.org,
mtosatti@redhat.com, will@kernel.org, peterz@infradead.org,
leon@sidebranch.com, linux-arm-kernel@lists.infradead.org,
pauld@redhat.com, netdev@vger.kernel.org
In-Reply-To: <8d887e59ca713726f4fcb25a316e1e932b02823e.camel@marvell.com>
From: Yuri Norov <ynorov@marvell.com>
Make sure that kick_all_cpus_sync() does not call CPUs that are running
isolated tasks.
Signed-off-by: Yuri Norov <ynorov@marvell.com>
[abelits@marvell.com: use safe task_isolation_cpumask() implementation]
Signed-off-by: Alex Belits <abelits@marvell.com>
---
kernel/smp.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/kernel/smp.c b/kernel/smp.c
index 4d17501433be..b2faecf58ed0 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -932,9 +932,21 @@ static void do_nothing(void *unused)
*/
void kick_all_cpus_sync(void)
{
+ struct cpumask mask;
+
/* Make sure the change is visible before we kick the cpus */
smp_mb();
- smp_call_function(do_nothing, NULL, 1);
+
+ preempt_disable();
+#ifdef CONFIG_TASK_ISOLATION
+ cpumask_clear(&mask);
+ task_isolation_cpumask(&mask);
+ cpumask_complement(&mask, &mask);
+#else
+ cpumask_setall(&mask);
+#endif
+ smp_call_function_many(&mask, do_nothing, NULL, 1);
+ preempt_enable();
}
EXPORT_SYMBOL_GPL(kick_all_cpus_sync);
--
2.20.1
^ permalink raw reply related
* [PATCH v5 8/9] task_isolation: ringbuffer: don't interrupt CPUs running isolated tasks on buffer resize
From: Alex Belits @ 2020-11-23 17:58 UTC (permalink / raw)
To: nitesh@redhat.com, frederic@kernel.org
Cc: Prasun Kapoor, linux-api@vger.kernel.org, davem@davemloft.net,
trix@redhat.com, mingo@kernel.org, catalin.marinas@arm.com,
rostedt@goodmis.org, linux-kernel@vger.kernel.org,
peterx@redhat.com, tglx@linutronix.de, linux-arch@vger.kernel.org,
mtosatti@redhat.com, will@kernel.org, peterz@infradead.org,
leon@sidebranch.com, linux-arm-kernel@lists.infradead.org,
pauld@redhat.com, netdev@vger.kernel.org
In-Reply-To: <8d887e59ca713726f4fcb25a316e1e932b02823e.camel@marvell.com>
From: Yuri Norov <ynorov@marvell.com>
CPUs running isolated tasks are in userspace, so they don't have to
perform ring buffer updates immediately. If ring_buffer_resize()
schedules the update on those CPUs, isolation is broken. To prevent
that, updates for CPUs running isolated tasks are performed locally,
like for offline CPUs.
A race condition between this update and isolation breaking is avoided
at the cost of disabling per_cpu buffer writing for the time of update
when it coincides with isolation breaking.
Signed-off-by: Yuri Norov <ynorov@marvell.com>
[abelits@marvell.com: updated to prevent race with isolation breaking]
Signed-off-by: Alex Belits <abelits@marvell.com>
---
kernel/trace/ring_buffer.c | 63 ++++++++++++++++++++++++++++++++++----
1 file changed, 57 insertions(+), 6 deletions(-)
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index dc83b3fa9fe7..9e4fb3ed2af0 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -21,6 +21,7 @@
#include <linux/delay.h>
#include <linux/slab.h>
#include <linux/init.h>
+#include <linux/isolation.h>
#include <linux/hash.h>
#include <linux/list.h>
#include <linux/cpu.h>
@@ -1939,6 +1940,38 @@ static void update_pages_handler(struct work_struct *work)
complete(&cpu_buffer->update_done);
}
+static bool update_if_isolated(struct ring_buffer_per_cpu *cpu_buffer,
+ int cpu)
+{
+ bool rv = false;
+
+ smp_rmb();
+ if (task_isolation_on_cpu(cpu)) {
+ /*
+ * CPU is running isolated task. Since it may lose
+ * isolation and re-enter kernel simultaneously with
+ * this update, disable recording until it's done.
+ */
+ atomic_inc(&cpu_buffer->record_disabled);
+ /* Make sure, update is done, and isolation state is current */
+ smp_mb();
+ if (task_isolation_on_cpu(cpu)) {
+ /*
+ * If CPU is still running isolated task, we
+ * can be sure that breaking isolation will
+ * happen while recording is disabled, and CPU
+ * will not touch this buffer until the update
+ * is done.
+ */
+ rb_update_pages(cpu_buffer);
+ cpu_buffer->nr_pages_to_update = 0;
+ rv = true;
+ }
+ atomic_dec(&cpu_buffer->record_disabled);
+ }
+ return rv;
+}
+
/**
* ring_buffer_resize - resize the ring buffer
* @buffer: the buffer to resize.
@@ -2028,13 +2061,22 @@ int ring_buffer_resize(struct trace_buffer *buffer, unsigned long size,
if (!cpu_buffer->nr_pages_to_update)
continue;
- /* Can't run something on an offline CPU. */
+ /*
+ * Can't run something on an offline CPU.
+ *
+ * CPUs running isolated tasks don't have to
+ * update ring buffers until they exit
+ * isolation because they are in
+ * userspace. Use the procedure that prevents
+ * race condition with isolation breaking.
+ */
if (!cpu_online(cpu)) {
rb_update_pages(cpu_buffer);
cpu_buffer->nr_pages_to_update = 0;
} else {
- schedule_work_on(cpu,
- &cpu_buffer->update_pages_work);
+ if (!update_if_isolated(cpu_buffer, cpu))
+ schedule_work_on(cpu,
+ &cpu_buffer->update_pages_work);
}
}
@@ -2083,13 +2125,22 @@ int ring_buffer_resize(struct trace_buffer *buffer, unsigned long size,
get_online_cpus();
- /* Can't run something on an offline CPU. */
+ /*
+ * Can't run something on an offline CPU.
+ *
+ * CPUs running isolated tasks don't have to update
+ * ring buffers until they exit isolation because they
+ * are in userspace. Use the procedure that prevents
+ * race condition with isolation breaking.
+ */
if (!cpu_online(cpu_id))
rb_update_pages(cpu_buffer);
else {
- schedule_work_on(cpu_id,
+ if (!update_if_isolated(cpu_buffer, cpu_id))
+ schedule_work_on(cpu_id,
&cpu_buffer->update_pages_work);
- wait_for_completion(&cpu_buffer->update_done);
+ wait_for_completion(&cpu_buffer->update_done);
+ }
}
cpu_buffer->nr_pages_to_update = 0;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net-next 0/2] TLS TX HW offload for Bond
From: Jakub Kicinski @ 2020-11-23 18:20 UTC (permalink / raw)
To: Tariq Toukan
Cc: Jarod Wilson, Tariq Toukan, David S. Miller, netdev,
Saeed Mahameed, Moshe Shemesh, Maor Gottlieb
In-Reply-To: <ee42de0e-e657-4108-3fb7-05e252979673@gmail.com>
On Sun, 22 Nov 2020 14:48:04 +0200 Tariq Toukan wrote:
> On 11/19/2020 6:38 PM, Jakub Kicinski wrote:
> > On Thu, 19 Nov 2020 17:59:38 +0200 Tariq Toukan wrote:
> >> On 11/19/2020 2:02 AM, Jakub Kicinski wrote:
> >>> On Sun, 15 Nov 2020 15:42:49 +0200 Tariq Toukan wrote:
> >>>> This series opens TLS TX HW offload for bond interfaces.
> >>>> This allows bond interfaces to benefit from capable slave devices.
> >>>>
> >>>> The first patch adds real_dev field in TLS context structure, and aligns
> >>>> usages in TLS module and supporting drivers.
> >>>> The second patch opens the offload for bond interfaces.
> >>>>
> >>>> For the configuration above, SW kTLS keeps picking the same slave
> >>>> To keep simple track of the HW and SW TLS contexts, we bind each socket to
> >>>> a specific slave for the socket's whole lifetime. This is logically valid
> >>>> (and similar to the SW kTLS behavior) in the following bond configuration,
> >>>> so we restrict the offload support to it:
> >>>>
> >>>> ((mode == balance-xor) or (mode == 802.3ad))
> >>>> and xmit_hash_policy == layer3+4.
> >>>
> >>> This does not feel extremely clean, maybe you can convince me otherwise.
> >>>
> >>> Can we extend netdev_get_xmit_slave() and figure out the output dev
> >>> (and if it's "stable") in a more generic way? And just feed that dev
> >>> into TLS handling?
> >>
> >> I don't see we go through netdev_get_xmit_slave(), but through
> >> .ndo_start_xmit (bond_start_xmit).
> >
> > I may be misunderstanding the purpose of netdev_get_xmit_slave(),
> > please correct me if I'm wrong. AFAIU it's supposed to return a
> > lower netdev that the skb should then be xmited on.
>
> That's true. It was recently added and used by the RDMA team. Not used
> or integrated in the Eth networking stack.
>
> > So what I was thinking was either construct an skb or somehow reshuffle
> > the netdev_get_xmit_slave() code to take a flow dissector output or
> > ${insert other ideas}. Then add a helper in the core that would drill
> > down from the socket netdev to the "egress" netdev. Have TLS call
> > that helper, and talk to the "egress" netdev from the start, rather
> > than the socket's netdev. Then loosen the checks on software devices.
>
> As I understand it, best if we can even generalize this to apply to all
> kinds of traffic: bond driver won't do the xmit itself anymore, it just
> picks an egress dev and returns it. The core infrastructure will call
> the xmit function for the egress dev.
I think you went way further than I was intending :) I was only
considering the control path. Leave the datapath unchanged.
AFAIK you're making 3 changes:
- forwarding tls ops
- pinning flows
- handling features
Pinning of the TLS device to a leg of the bond looks like ~15LoC.
I think we can live with that.
It's the 150 LoC of forwarding TLS ops and duplicating dev selection
logic in bond_sk_hash_l34() that I'd rather avoid.
Handling features is probably fine, too, I haven't thought about that
much.
> I like the idea, it can generalize code structures for all kinds of
> upper-devices and sockets, taking them into a common place in core,
> which reduces code duplications.
>
> If we go only half the way, i.e. keep xmit logic in bond for
> non-TLS-offloaded traffic, then we have to let TLS module (and others in
> the future) act deferentially for different kinds of devs (upper/lower)
> which IMHO reduces generality.
How so? I was expecting TLS to just do something like:
netdev = sk_get_xmit_dev_lowest(sk);
which would recursively call get_xmit_slave(CONST) until it reaches
a device which doesn't resolve further.
BTW is the flow pinning to bond legs actually a must-do? I don't know
much about bonding but wouldn't that mean that if the selected leg goes
down we'd lose connectivity, rather than falling back to SW crypto?
> I'm in favor of the deeper change. It will be on a larger scale, and
> totally orthogonal to the current TLS offload support in bond.
>
> If we decide to apply the idea only to TLS sockets (or any subset of
> sockets) we're actually taking a generic one-flow (the xmit patch of a
> bond dev) and turning it into two (or potentially more) flows, depending
> on the socket type. This also reduces generality.
I don't follow this part.
> > I'm probably missing the problem you're trying to explain to me :S
>
> I kept the patch minimal, and kept the TLS offload logic internal to the
> bond driver, just like it is internal to the device drivers (mlx5e, and
> others), with no core infrastructure modification.
>
> > Side note - Jarod, I'd be happy to take a patch renaming
> > netdev_get_xmit_slave() and the ndo, if you have the cycles to send
> > a patch. It's a recent addition, and in the core we should make more
> > of an effort to avoid sensitive terms.
> >
> >> Currently I have my check there to
> >> catch all skbs belonging to offloaded TLS sockets.
> >>
> >> The TLS offload get_slave() logic decision is per socket, so the result
> >> cannot be saved in the bond memory. Currently I save the real_dev field
> >> in the TLS context structure.
> >
> > Right, but we could just have ctx->netdev be the "egress" netdev
> > always, right? Do we expect somewhere that it's going to be matching
> > the socket's dst?
>
> So once the offload context is established we totally bypass the bond
> dev? and lose interaction or reference to it?
Yup, I don't think we need it.
> What if the egress dev is detached form the bond? We must then be
> notified somehow.
Do we notify TLS when routing changes? I think it's a separate topic.
If we have the code to "un-offload" a flow we could handle clearing
features better and notify from sk_validate_xmit_skb that the flow
started hitting unexpected dev, hence it should be re-offloaded.
I don't think we need an explicit invalidation from the particular
drivers here.
^ permalink raw reply
* [net-next v3 4/8] seg6: add callbacks for customizing the creation/destruction of a behavior
From: Andrea Mayer @ 2020-11-23 18:28 UTC (permalink / raw)
To: David S. Miller, David Ahern, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Jakub Kicinski, Shuah Khan, Shrijeet Mukherjee,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, netdev, linux-kernel, linux-kselftest
Cc: Nathan Chancellor, Stefano Salsano, Paolo Lungaroni,
Ahmed Abdelsalam, Andrea Mayer
In-Reply-To: <20201123182857.4640-1-andrea.mayer@uniroma2.it>
We introduce two callbacks used for customizing the creation/destruction of
a SRv6 behavior. Such callbacks are defined in the new struct
seg6_local_lwtunnel_ops and hereafter we provide a brief description of
them:
- build_state(...): used for calling the custom constructor of the
behavior during its initialization phase and after all the attributes
have been parsed successfully;
- destroy_state(...): used for calling the custom destructor of the
behavior before it is completely destroyed.
Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
---
net/ipv6/seg6_local.c | 49 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
index 3b5657c622a0..da5bf4167a52 100644
--- a/net/ipv6/seg6_local.c
+++ b/net/ipv6/seg6_local.c
@@ -33,6 +33,13 @@
struct seg6_local_lwt;
+/* callbacks used for customizing the creation and destruction of a behavior */
+struct seg6_local_lwtunnel_ops {
+ int (*build_state)(struct seg6_local_lwt *slwt, const void *cfg,
+ struct netlink_ext_ack *extack);
+ void (*destroy_state)(struct seg6_local_lwt *slwt);
+};
+
struct seg6_action_desc {
int action;
unsigned long attrs;
@@ -53,6 +60,8 @@ struct seg6_action_desc {
int (*input)(struct sk_buff *skb, struct seg6_local_lwt *slwt);
int static_headroom;
+
+ struct seg6_local_lwtunnel_ops slwt_ops;
};
struct bpf_lwt_prog {
@@ -1055,6 +1064,38 @@ static int parse_nla_optional_attrs(struct nlattr **attrs,
return err;
}
+/* call the custom constructor of the behavior during its initialization phase
+ * and after that all its attributes have been parsed successfully.
+ */
+static int
+seg6_local_lwtunnel_build_state(struct seg6_local_lwt *slwt, const void *cfg,
+ struct netlink_ext_ack *extack)
+{
+ struct seg6_action_desc *desc = slwt->desc;
+ struct seg6_local_lwtunnel_ops *ops;
+
+ ops = &desc->slwt_ops;
+ if (!ops->build_state)
+ return 0;
+
+ return ops->build_state(slwt, cfg, extack);
+}
+
+/* call the custom destructor of the behavior which is invoked before the
+ * tunnel is going to be destroyed.
+ */
+static void seg6_local_lwtunnel_destroy_state(struct seg6_local_lwt *slwt)
+{
+ struct seg6_action_desc *desc = slwt->desc;
+ struct seg6_local_lwtunnel_ops *ops;
+
+ ops = &desc->slwt_ops;
+ if (!ops->destroy_state)
+ return;
+
+ ops->destroy_state(slwt);
+}
+
static int parse_nla_action(struct nlattr **attrs, struct seg6_local_lwt *slwt)
{
struct seg6_action_param *param;
@@ -1154,6 +1195,10 @@ static int seg6_local_build_state(struct net *net, struct nlattr *nla,
if (err < 0)
goto out_free;
+ err = seg6_local_lwtunnel_build_state(slwt, cfg, extack);
+ if (err < 0)
+ goto out_destroy_attrs;
+
newts->type = LWTUNNEL_ENCAP_SEG6_LOCAL;
newts->flags = LWTUNNEL_STATE_INPUT_REDIRECT;
newts->headroom = slwt->headroom;
@@ -1162,6 +1207,8 @@ static int seg6_local_build_state(struct net *net, struct nlattr *nla,
return 0;
+out_destroy_attrs:
+ destroy_attrs(slwt);
out_free:
kfree(newts);
return err;
@@ -1171,6 +1218,8 @@ static void seg6_local_destroy_state(struct lwtunnel_state *lwt)
{
struct seg6_local_lwt *slwt = seg6_local_lwtunnel(lwt);
+ seg6_local_lwtunnel_destroy_state(slwt);
+
destroy_attrs(slwt);
return;
--
2.20.1
^ permalink raw reply related
* [iproute2-next v1 1/1] seg6: add support for vrftable attribute in End.DT4/DT6 behaviors
From: Andrea Mayer @ 2020-11-23 18:28 UTC (permalink / raw)
To: David S. Miller, David Ahern, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Jakub Kicinski, Shuah Khan, Shrijeet Mukherjee,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, netdev, linux-kernel, linux-kselftest
Cc: Nathan Chancellor, Stefano Salsano, Paolo Lungaroni,
Ahmed Abdelsalam, Andrea Mayer
In-Reply-To: <20201123182857.4640-1-andrea.mayer@uniroma2.it>
introduces the "vrftable" attribute for supporting the End.DT4 and
End.DT6 behaviors in iproute2.
The "vrftable" attribute indicates the routing table associated with
the VRF device used by SRv6 End.DT4/DT6 for routing IPv4/IPv6 packets.
The End.DT4/DT6 is used to implement IPv4/IPv6 L3 VPNs based on Segment
Routing over IPv6 networks in multi-tenants environments.
It decapsulates the received packets and it performs the IPv4/IPv6 routing
lookup in the routing table of the tenant.
The End.DT4/DT6 leverages a VRF device in order to force the routing
lookup into the associated routing table using the "vrftable" attribute.
Some examples:
$ ip -6 route add 2001:db8::1 encap seg6local action End.DT4 vrftable 100 dev eth0
$ ip -6 route add 2001:db8::2 encap seg6local action End.DT6 vrftable 200 dev eth0
Standard Output:
$ ip -6 route show 2001:db8::1
2001:db8::1 encap seg6local action End.DT4 vrftable 100 dev eth0 metric 1024 pref medium
JSON Output:
$ ip -6 -j -p route show 2001:db8::2
[ {
"dst": "2001:db8::2",
"encap": "seg6local",
"action": "End.DT6",
"vrftable": 200,
"dev": "eth0",
"metric": 1024,
"flags": [ ],
"pref": "medium"
} ]
Signed-off-by: Paolo Lungaroni <paolo.lungaroni@cnit.it>
Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
---
include/uapi/linux/seg6_local.h | 1 +
ip/iproute_lwtunnel.c | 19 ++++++++++++++++---
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/include/uapi/linux/seg6_local.h b/include/uapi/linux/seg6_local.h
index 5312de80..bb5c8ddf 100644
--- a/include/uapi/linux/seg6_local.h
+++ b/include/uapi/linux/seg6_local.h
@@ -26,6 +26,7 @@ enum {
SEG6_LOCAL_IIF,
SEG6_LOCAL_OIF,
SEG6_LOCAL_BPF,
+ SEG6_LOCAL_VRFTABLE,
__SEG6_LOCAL_MAX,
};
#define SEG6_LOCAL_MAX (__SEG6_LOCAL_MAX - 1)
diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c
index 9b4f0885..1ab95cd2 100644
--- a/ip/iproute_lwtunnel.c
+++ b/ip/iproute_lwtunnel.c
@@ -294,6 +294,11 @@ static void print_encap_seg6local(FILE *fp, struct rtattr *encap)
rtnl_rttable_n2a(rta_getattr_u32(tb[SEG6_LOCAL_TABLE]),
b1, sizeof(b1)));
+ if (tb[SEG6_LOCAL_VRFTABLE])
+ print_string(PRINT_ANY, "vrftable", "vrftable %s ",
+ rtnl_rttable_n2a(rta_getattr_u32(tb[SEG6_LOCAL_VRFTABLE]),
+ b1, sizeof(b1)));
+
if (tb[SEG6_LOCAL_NH4]) {
print_string(PRINT_ANY, "nh4",
"nh4 %s ", rt_addr_n2a_rta(AF_INET, tb[SEG6_LOCAL_NH4]));
@@ -860,9 +865,10 @@ static int lwt_parse_bpf(struct rtattr *rta, size_t len,
static int parse_encap_seg6local(struct rtattr *rta, size_t len, int *argcp,
char ***argvp)
{
- int segs_ok = 0, hmac_ok = 0, table_ok = 0, nh4_ok = 0, nh6_ok = 0;
- int iif_ok = 0, oif_ok = 0, action_ok = 0, srh_ok = 0, bpf_ok = 0;
- __u32 action = 0, table, iif, oif;
+ int segs_ok = 0, hmac_ok = 0, table_ok = 0, vrftable_ok = 0;
+ int nh4_ok = 0, nh6_ok = 0, iif_ok = 0, oif_ok = 0;
+ __u32 action = 0, table, vrftable, iif, oif;
+ int action_ok = 0, srh_ok = 0, bpf_ok = 0;
struct ipv6_sr_hdr *srh;
char **argv = *argvp;
int argc = *argcp;
@@ -887,6 +893,13 @@ static int parse_encap_seg6local(struct rtattr *rta, size_t len, int *argcp,
duparg2("table", *argv);
rtnl_rttable_a2n(&table, *argv);
ret = rta_addattr32(rta, len, SEG6_LOCAL_TABLE, table);
+ } else if (strcmp(*argv, "vrftable") == 0) {
+ NEXT_ARG();
+ if (vrftable_ok++)
+ duparg2("vrftable", *argv);
+ rtnl_rttable_a2n(&vrftable, *argv);
+ ret = rta_addattr32(rta, len, SEG6_LOCAL_VRFTABLE,
+ vrftable);
} else if (strcmp(*argv, "nh4") == 0) {
NEXT_ARG();
if (nh4_ok++)
--
2.20.1
^ permalink raw reply related
* [net-next v3 5/8] seg6: add support for the SRv6 End.DT4 behavior
From: Andrea Mayer @ 2020-11-23 18:28 UTC (permalink / raw)
To: David S. Miller, David Ahern, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Jakub Kicinski, Shuah Khan, Shrijeet Mukherjee,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, netdev, linux-kernel, linux-kselftest
Cc: Nathan Chancellor, Stefano Salsano, Paolo Lungaroni,
Ahmed Abdelsalam, Andrea Mayer
In-Reply-To: <20201123182857.4640-1-andrea.mayer@uniroma2.it>
SRv6 End.DT4 is defined in the SRv6 Network Programming [1].
The SRv6 End.DT4 is used to implement IPv4 L3VPN use-cases in
multi-tenants environments. It decapsulates the received packets and it
performs IPv4 routing lookup in the routing table of the tenant.
The SRv6 End.DT4 Linux implementation leverages a VRF device in order to
force the routing lookup into the associated routing table.
To make the End.DT4 work properly, it must be guaranteed that the routing
table used for routing lookup operations is bound to one and only one
VRF during the tunnel creation. Such constraint has to be enforced by
enabling the VRF strict_mode sysctl parameter, i.e:
$ sysctl -wq net.vrf.strict_mode=1.
At JANOG44, LINE corporation presented their multi-tenant DC architecture
using SRv6 [2]. In the slides, they reported that the Linux kernel is
missing the support of SRv6 End.DT4 behavior.
The SRv6 End.DT4 behavior can be instantiated using a command similar to
the following:
$ ip route add 2001:db8::1 encap seg6local action End.DT4 vrftable 100 dev eth0
We introduce the "vrftable" extension in iproute2 in a following patch.
[1] https://tools.ietf.org/html/draft-ietf-spring-srv6-network-programming
[2] https://speakerdeck.com/line_developers/line-data-center-networking-with-srv6
Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
---
include/uapi/linux/seg6_local.h | 1 +
net/ipv6/seg6_local.c | 290 ++++++++++++++++++++++++++++++++
2 files changed, 291 insertions(+)
diff --git a/include/uapi/linux/seg6_local.h b/include/uapi/linux/seg6_local.h
index edc138bdc56d..3b39ef1dbb46 100644
--- a/include/uapi/linux/seg6_local.h
+++ b/include/uapi/linux/seg6_local.h
@@ -26,6 +26,7 @@ enum {
SEG6_LOCAL_IIF,
SEG6_LOCAL_OIF,
SEG6_LOCAL_BPF,
+ SEG6_LOCAL_VRFTABLE,
__SEG6_LOCAL_MAX,
};
#define SEG6_LOCAL_MAX (__SEG6_LOCAL_MAX - 1)
diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
index da5bf4167a52..117405985e6f 100644
--- a/net/ipv6/seg6_local.c
+++ b/net/ipv6/seg6_local.c
@@ -69,6 +69,28 @@ struct bpf_lwt_prog {
char *name;
};
+enum seg6_end_dt_mode {
+ DT_INVALID_MODE = -EINVAL,
+ DT_LEGACY_MODE = 0,
+ DT_VRF_MODE = 1,
+};
+
+struct seg6_end_dt_info {
+ enum seg6_end_dt_mode mode;
+
+ struct net *net;
+ /* VRF device associated to the routing table used by the SRv6
+ * End.DT4/DT6 behavior for routing IPv4/IPv6 packets.
+ */
+ int vrf_ifindex;
+ int vrf_table;
+
+ /* tunneled packet proto and family (IPv4 or IPv6) */
+ __be16 proto;
+ u16 family;
+ int hdrlen;
+};
+
struct seg6_local_lwt {
int action;
struct ipv6_sr_hdr *srh;
@@ -78,6 +100,9 @@ struct seg6_local_lwt {
int iif;
int oif;
struct bpf_lwt_prog bpf;
+#ifdef CONFIG_NET_L3_MASTER_DEV
+ struct seg6_end_dt_info dt_info;
+#endif
int headroom;
struct seg6_action_desc *desc;
@@ -429,6 +454,203 @@ static int input_action_end_dx4(struct sk_buff *skb,
return -EINVAL;
}
+#ifdef CONFIG_NET_L3_MASTER_DEV
+static struct net *fib6_config_get_net(const struct fib6_config *fib6_cfg)
+{
+ const struct nl_info *nli = &fib6_cfg->fc_nlinfo;
+
+ return nli->nl_net;
+}
+
+static int __seg6_end_dt_vrf_build(struct seg6_local_lwt *slwt, const void *cfg,
+ u16 family, struct netlink_ext_ack *extack)
+{
+ struct seg6_end_dt_info *info = &slwt->dt_info;
+ int vrf_ifindex;
+ struct net *net;
+
+ net = fib6_config_get_net(cfg);
+
+ /* note that vrf_table was already set by parse_nla_vrftable() */
+ vrf_ifindex = l3mdev_ifindex_lookup_by_table_id(L3MDEV_TYPE_VRF, net,
+ info->vrf_table);
+ if (vrf_ifindex < 0) {
+ if (vrf_ifindex == -EPERM) {
+ NL_SET_ERR_MSG(extack,
+ "Strict mode for VRF is disabled");
+ } else if (vrf_ifindex == -ENODEV) {
+ NL_SET_ERR_MSG(extack,
+ "Table has no associated VRF device");
+ } else {
+ pr_debug("seg6local: SRv6 End.DT* creation error=%d\n",
+ vrf_ifindex);
+ }
+
+ return vrf_ifindex;
+ }
+
+ info->net = net;
+ info->vrf_ifindex = vrf_ifindex;
+
+ switch (family) {
+ case AF_INET:
+ info->proto = htons(ETH_P_IP);
+ info->hdrlen = sizeof(struct iphdr);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ info->family = family;
+ info->mode = DT_VRF_MODE;
+
+ return 0;
+}
+
+/* The SRv6 End.DT4/DT6 behavior extracts the inner (IPv4/IPv6) packet and
+ * routes the IPv4/IPv6 packet by looking at the configured routing table.
+ *
+ * In the SRv6 End.DT4/DT6 use case, we can receive traffic (IPv6+Segment
+ * Routing Header packets) from several interfaces and the outer IPv6
+ * destination address (DA) is used for retrieving the specific instance of the
+ * End.DT4/DT6 behavior that should process the packets.
+ *
+ * However, the inner IPv4/IPv6 packet is not really bound to any receiving
+ * interface and thus the End.DT4/DT6 sets the VRF (associated with the
+ * corresponding routing table) as the *receiving* interface.
+ * In other words, the End.DT4/DT6 processes a packet as if it has been received
+ * directly by the VRF (and not by one of its slave devices, if any).
+ * In this way, the VRF interface is used for routing the IPv4/IPv6 packet in
+ * according to the routing table configured by the End.DT4/DT6 instance.
+ *
+ * This design allows you to get some interesting features like:
+ * 1) the statistics on rx packets;
+ * 2) the possibility to install a packet sniffer on the receiving interface
+ * (the VRF one) for looking at the incoming packets;
+ * 3) the possibility to leverage the netfilter prerouting hook for the inner
+ * IPv4 packet.
+ *
+ * This function returns:
+ * - the sk_buff* when the VRF rcv handler has processed the packet correctly;
+ * - NULL when the skb is consumed by the VRF rcv handler;
+ * - a pointer which encodes a negative error number in case of error.
+ * Note that in this case, the function takes care of freeing the skb.
+ */
+static struct sk_buff *end_dt_vrf_rcv(struct sk_buff *skb, u16 family,
+ struct net_device *dev)
+{
+ /* based on l3mdev_ip_rcv; we are only interested in the master */
+ if (unlikely(!netif_is_l3_master(dev) && !netif_has_l3_rx_handler(dev)))
+ goto drop;
+
+ if (unlikely(!dev->l3mdev_ops->l3mdev_l3_rcv))
+ goto drop;
+
+ /* the decap packet IPv4/IPv6 does not come with any mac header info.
+ * We must unset the mac header to allow the VRF device to rebuild it,
+ * just in case there is a sniffer attached on the device.
+ */
+ skb_unset_mac_header(skb);
+
+ skb = dev->l3mdev_ops->l3mdev_l3_rcv(dev, skb, family);
+ if (!skb)
+ /* the skb buffer was consumed by the handler */
+ return NULL;
+
+ /* when a packet is received by a VRF or by one of its slaves, the
+ * master device reference is set into the skb.
+ */
+ if (unlikely(skb->dev != dev || skb->skb_iif != dev->ifindex))
+ goto drop;
+
+ return skb;
+
+drop:
+ kfree_skb(skb);
+ return ERR_PTR(-EINVAL);
+}
+
+static struct net_device *end_dt_get_vrf_rcu(struct sk_buff *skb,
+ struct seg6_end_dt_info *info)
+{
+ int vrf_ifindex = info->vrf_ifindex;
+ struct net *net = info->net;
+
+ if (unlikely(vrf_ifindex < 0))
+ goto error;
+
+ if (unlikely(!net_eq(dev_net(skb->dev), net)))
+ goto error;
+
+ return dev_get_by_index_rcu(net, vrf_ifindex);
+
+error:
+ return NULL;
+}
+
+static struct sk_buff *end_dt_vrf_core(struct sk_buff *skb,
+ struct seg6_local_lwt *slwt)
+{
+ struct seg6_end_dt_info *info = &slwt->dt_info;
+ struct net_device *vrf;
+
+ vrf = end_dt_get_vrf_rcu(skb, info);
+ if (unlikely(!vrf))
+ goto drop;
+
+ skb->protocol = info->proto;
+
+ skb_dst_drop(skb);
+
+ skb_set_transport_header(skb, info->hdrlen);
+
+ return end_dt_vrf_rcv(skb, info->family, vrf);
+
+drop:
+ kfree_skb(skb);
+ return ERR_PTR(-EINVAL);
+}
+
+static int input_action_end_dt4(struct sk_buff *skb,
+ struct seg6_local_lwt *slwt)
+{
+ struct iphdr *iph;
+ int err;
+
+ if (!decap_and_validate(skb, IPPROTO_IPIP))
+ goto drop;
+
+ if (!pskb_may_pull(skb, sizeof(struct iphdr)))
+ goto drop;
+
+ skb = end_dt_vrf_core(skb, slwt);
+ if (!skb)
+ /* packet has been processed and consumed by the VRF */
+ return 0;
+
+ if (IS_ERR(skb))
+ return PTR_ERR(skb);
+
+ iph = ip_hdr(skb);
+
+ err = ip_route_input(skb, iph->daddr, iph->saddr, 0, skb->dev);
+ if (unlikely(err))
+ goto drop;
+
+ return dst_input(skb);
+
+drop:
+ kfree_skb(skb);
+ return -EINVAL;
+}
+
+static int seg6_end_dt4_build(struct seg6_local_lwt *slwt, const void *cfg,
+ struct netlink_ext_ack *extack)
+{
+ return __seg6_end_dt_vrf_build(slwt, cfg, AF_INET, extack);
+}
+#endif
+
static int input_action_end_dt6(struct sk_buff *skb,
struct seg6_local_lwt *slwt)
{
@@ -617,6 +839,16 @@ static struct seg6_action_desc seg6_action_table[] = {
.attrs = (1 << SEG6_LOCAL_NH4),
.input = input_action_end_dx4,
},
+ {
+ .action = SEG6_LOCAL_ACTION_END_DT4,
+ .attrs = (1 << SEG6_LOCAL_VRFTABLE),
+#ifdef CONFIG_NET_L3_MASTER_DEV
+ .input = input_action_end_dt4,
+ .slwt_ops = {
+ .build_state = seg6_end_dt4_build,
+ },
+#endif
+ },
{
.action = SEG6_LOCAL_ACTION_END_DT6,
.attrs = (1 << SEG6_LOCAL_TABLE),
@@ -677,6 +909,7 @@ static const struct nla_policy seg6_local_policy[SEG6_LOCAL_MAX + 1] = {
[SEG6_LOCAL_ACTION] = { .type = NLA_U32 },
[SEG6_LOCAL_SRH] = { .type = NLA_BINARY },
[SEG6_LOCAL_TABLE] = { .type = NLA_U32 },
+ [SEG6_LOCAL_VRFTABLE] = { .type = NLA_U32 },
[SEG6_LOCAL_NH4] = { .type = NLA_BINARY,
.len = sizeof(struct in_addr) },
[SEG6_LOCAL_NH6] = { .type = NLA_BINARY,
@@ -766,6 +999,56 @@ static int cmp_nla_table(struct seg6_local_lwt *a, struct seg6_local_lwt *b)
return 0;
}
+static struct
+seg6_end_dt_info *seg6_possible_end_dt_info(struct seg6_local_lwt *slwt)
+{
+#ifdef CONFIG_NET_L3_MASTER_DEV
+ return &slwt->dt_info;
+#else
+ return ERR_PTR(-EOPNOTSUPP);
+#endif
+}
+
+static int parse_nla_vrftable(struct nlattr **attrs,
+ struct seg6_local_lwt *slwt)
+{
+ struct seg6_end_dt_info *info = seg6_possible_end_dt_info(slwt);
+
+ if (IS_ERR(info))
+ return PTR_ERR(info);
+
+ info->vrf_table = nla_get_u32(attrs[SEG6_LOCAL_VRFTABLE]);
+
+ return 0;
+}
+
+static int put_nla_vrftable(struct sk_buff *skb, struct seg6_local_lwt *slwt)
+{
+ struct seg6_end_dt_info *info = seg6_possible_end_dt_info(slwt);
+
+ if (IS_ERR(info))
+ return PTR_ERR(info);
+
+ if (nla_put_u32(skb, SEG6_LOCAL_VRFTABLE, info->vrf_table))
+ return -EMSGSIZE;
+
+ return 0;
+}
+
+static int cmp_nla_vrftable(struct seg6_local_lwt *a, struct seg6_local_lwt *b)
+{
+ struct seg6_end_dt_info *info_a = seg6_possible_end_dt_info(a);
+ struct seg6_end_dt_info *info_b = seg6_possible_end_dt_info(b);
+
+ if (IS_ERR(info_a) || IS_ERR(info_b))
+ return 1;
+
+ if (info_a->vrf_table != info_b->vrf_table)
+ return 1;
+
+ return 0;
+}
+
static int parse_nla_nh4(struct nlattr **attrs, struct seg6_local_lwt *slwt)
{
memcpy(&slwt->nh4, nla_data(attrs[SEG6_LOCAL_NH4]),
@@ -984,6 +1267,10 @@ static struct seg6_action_param seg6_action_params[SEG6_LOCAL_MAX + 1] = {
.cmp = cmp_nla_bpf,
.destroy = destroy_attr_bpf },
+ [SEG6_LOCAL_VRFTABLE] = { .parse = parse_nla_vrftable,
+ .put = put_nla_vrftable,
+ .cmp = cmp_nla_vrftable },
+
};
/* call the destroy() callback (if available) for each set attribute in
@@ -1283,6 +1570,9 @@ static int seg6_local_get_encap_size(struct lwtunnel_state *lwt)
nla_total_size(MAX_PROG_NAME) +
nla_total_size(4);
+ if (attrs & (1 << SEG6_LOCAL_VRFTABLE))
+ nlsize += nla_total_size(4);
+
return nlsize;
}
--
2.20.1
^ permalink raw reply related
* [net-next v3 0/8] seg6: add support for SRv6 End.DT4/DT6 behavior
From: Andrea Mayer @ 2020-11-23 18:28 UTC (permalink / raw)
To: David S. Miller, David Ahern, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Jakub Kicinski, Shuah Khan, Shrijeet Mukherjee,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, netdev, linux-kernel, linux-kselftest
Cc: Nathan Chancellor, Stefano Salsano, Paolo Lungaroni,
Ahmed Abdelsalam, Andrea Mayer
This patchset provides support for the SRv6 End.DT4 and SRv6 End.DT6 (VRF mode)
behavior.
The SRv6 End.DT4 is used to implement multi-tenant IPv4 L3 VPN. It decapsulates
the received packets and performs IPv4 routing lookup in the routing table of
the tenant. The SRv6 End.DT4 Linux implementation leverages a VRF device. The
SRv6 End.DT4 is defined in the SRv6 Network Programming [1].
The Linux kernel already offers an implementation of the SRv6 End.DT6
behavior which permits IPv6 L3 VPNs over SRv6 networks. This new
implementation of DT6 is based on the same VRF infrastructure already
exploited for implementing the SRv6 End.DT4 behavior. The aim of the new
SRv6 End.DT6 in VRF mode consists in simplifying the construction of IPv6
L3 VPN services in the multi-tenant environment.
Currently, the two SRv6 End.DT6 implementations (legacy and VRF mode)
coexist seamlessly and can be chosen according to the context and the user
preferences.
- Patch 1 is needed to solve a pre-existing issue with tunneled packets
when a sniffer is attached;
- Patch 2 improves the management of the seg6local attributes used by the
SRv6 behaviors;
- Patch 3 adds support for optional attributes in SRv6 behaviors;
- Patch 4 introduces two callbacks used for customizing the
creation/destruction of a SRv6 behavior;
- Patch 5 is the core patch that adds support for the SRv6 End.DT4
behavior;
- Patch 6 introduces the VRF support for SRv6 End.DT6 behavior;
- Patch 7 adds the selftest for SRv6 End.DT4 behavior;
- Patch 8 adds the selftest for SRv6 End.DT6 (VRF mode) behavior;
- Patch 9 adds the vrftable attribute for End.DT4/DT6 behaviors in iproute2.
I would like to thank David Ahern for his support during the development of
this patchset.
Comments, suggestions and improvements are very welcome!
Thanks,
Andrea Mayer
v3
notes about the build bot:
- apparently the ',' (comma) in the subject prefix confused the build bot.
Removed the ',' in favor of ' ' (space).
Thanks to David Ahern and Konstantin Ryabitsev for shedding light on this
fact.
Thanks also to Nathan Chancellor for trying to build the patchset v2 by
simulating the bot issue.
add new patch for iproute2:
- [9/9] seg6: add support for vrftable attribute in End.DT4/DT6 behaviors
add new patch:
- [8/9] selftests: add selftest for the SRv6 End.DT6 (VRF) behavior
add new patch:
- [6/9] seg6: add VRF support for SRv6 End.DT6 behavior
add new patch:
- [3/9] seg6: add support for optional attributes in SRv6 behaviors
selftests: add selftest for the SRv6 End.DT4 behavior
- keep David Ahern's review tag since the code wasn't changed. Thanks to David
Ahern for his review.
seg6: add support for the SRv6 End.DT4 behavior
- remove useless error in seg6_end_dt4_build();
- remove #ifdef/#endif stubs for DT4 when CONFIG_NET_L3_MASTER_DEV is not
defined;
- fix coding style.
Thanks to Jakub Kicinski for his review and for all his suggestions.
seg6: add callbacks for customizing the creation/destruction of a behavior
- remove typedef(s) slwt_{build/destroy}_state_t;
- fix coding style: remove empty lines, trivial comments and rename labels in
the seg6_local_build_state() function.
Thanks to Jakub Kicinski for his review and for all his suggestions.
seg6: improve management of behavior attributes
- remove defensive programming approach in destroy_attr_srh(),
destroy_attr_bpf() and destroy_attrs();
- change the __destroy_attrs() function signature, renaming the 'end' argument
'parsed_max'. Now, the __destroy_attrs() keeps only the 'parsed_max' and
'slwt' arguments.
Thanks to Jakub Kicinski for his review and for all his suggestions.
vrf: add mac header for tunneled packets when sniffer is attached
- keep David Ahern's review tag since the code wasn't changed.
Thanks to Jakub Kicinski for pointing it out and David Ahern for his review.
v2
no changes made: resubmitted after false build report.
v1
improve comments;
add new patch 2/5 titled: seg6: improve management of behavior attributes
seg6: add support for the SRv6 End.DT4 behavior
- remove the inline keyword in the definition of fib6_config_get_net().
selftests: add selftest for the SRv6 End.DT4 behavior
- add check for the vrf sysctl
[1] https://tools.ietf.org/html/draft-ietf-spring-srv6-network-programming
Andrea Mayer (8):
vrf: add mac header for tunneled packets when sniffer is attached
seg6: improve management of behavior attributes
seg6: add support for optional attributes in SRv6 behaviors
seg6: add callbacks for customizing the creation/destruction of a
behavior
seg6: add support for the SRv6 End.DT4 behavior
seg6: add VRF support for SRv6 End.DT6 behavior
selftests: add selftest for the SRv6 End.DT4 behavior
selftests: add selftest for the SRv6 End.DT6 (VRF) behavior
drivers/net/vrf.c | 78 ++-
include/uapi/linux/seg6_local.h | 1 +
net/ipv6/seg6_local.c | 593 +++++++++++++++++-
.../selftests/net/srv6_end_dt4_l3vpn_test.sh | 494 +++++++++++++++
.../selftests/net/srv6_end_dt6_l3vpn_test.sh | 502 +++++++++++++++
5 files changed, 1649 insertions(+), 19 deletions(-)
create mode 100755 tools/testing/selftests/net/srv6_end_dt4_l3vpn_test.sh
create mode 100755 tools/testing/selftests/net/srv6_end_dt6_l3vpn_test.sh
Paolo Lungaroni (1):
seg6: add support for vrftable attribute in End.DT4/DT6 behaviors
include/uapi/linux/seg6_local.h | 1 +
ip/iproute_lwtunnel.c | 19 ++++++++++++++++---
2 files changed, 17 insertions(+), 3 deletions(-)
--
2.20.1
^ permalink raw reply
* [net-next v3 1/8] vrf: add mac header for tunneled packets when sniffer is attached
From: Andrea Mayer @ 2020-11-23 18:28 UTC (permalink / raw)
To: David S. Miller, David Ahern, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Jakub Kicinski, Shuah Khan, Shrijeet Mukherjee,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, netdev, linux-kernel, linux-kselftest
Cc: Nathan Chancellor, Stefano Salsano, Paolo Lungaroni,
Ahmed Abdelsalam, Andrea Mayer
In-Reply-To: <20201123182857.4640-1-andrea.mayer@uniroma2.it>
Before this patch, a sniffer attached to a VRF used as the receiving
interface of L3 tunneled packets detects them as malformed packets and
it complains about that (i.e.: tcpdump shows bogus packets).
The reason is that a tunneled L3 packet does not carry any L2
information and when the VRF is set as the receiving interface of a
decapsulated L3 packet, no mac header is currently set or valid.
Therefore, the purpose of this patch consists of adding a MAC header to
any packet which is directly received on the VRF interface ONLY IF:
i) a sniffer is attached on the VRF and ii) the mac header is not set.
In this case, the mac address of the VRF is copied in both the
destination and the source address of the ethernet header. The protocol
type is set either to IPv4 or IPv6, depending on which L3 packet is
received.
Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
Reviewed-by: David Ahern <dsahern@kernel.org>
---
drivers/net/vrf.c | 78 +++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 72 insertions(+), 6 deletions(-)
diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index f2793ffde191..51de12874717 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -1309,6 +1309,61 @@ static void vrf_ip6_input_dst(struct sk_buff *skb, struct net_device *vrf_dev,
skb_dst_set(skb, &rt6->dst);
}
+static int vrf_prepare_mac_header(struct sk_buff *skb,
+ struct net_device *vrf_dev, u16 proto)
+{
+ struct ethhdr *eth;
+ int err;
+
+ /* in general, we do not know if there is enough space in the head of
+ * the packet for hosting the mac header.
+ */
+ err = skb_cow_head(skb, LL_RESERVED_SPACE(vrf_dev));
+ if (unlikely(err))
+ /* no space in the skb head */
+ return -ENOBUFS;
+
+ __skb_push(skb, ETH_HLEN);
+ eth = (struct ethhdr *)skb->data;
+
+ skb_reset_mac_header(skb);
+
+ /* we set the ethernet destination and the source addresses to the
+ * address of the VRF device.
+ */
+ ether_addr_copy(eth->h_dest, vrf_dev->dev_addr);
+ ether_addr_copy(eth->h_source, vrf_dev->dev_addr);
+ eth->h_proto = htons(proto);
+
+ /* the destination address of the Ethernet frame corresponds to the
+ * address set on the VRF interface; therefore, the packet is intended
+ * to be processed locally.
+ */
+ skb->protocol = eth->h_proto;
+ skb->pkt_type = PACKET_HOST;
+
+ skb_postpush_rcsum(skb, skb->data, ETH_HLEN);
+
+ skb_pull_inline(skb, ETH_HLEN);
+
+ return 0;
+}
+
+/* prepare and add the mac header to the packet if it was not set previously.
+ * In this way, packet sniffers such as tcpdump can parse the packet correctly.
+ * If the mac header was already set, the original mac header is left
+ * untouched and the function returns immediately.
+ */
+static int vrf_add_mac_header_if_unset(struct sk_buff *skb,
+ struct net_device *vrf_dev,
+ u16 proto)
+{
+ if (skb_mac_header_was_set(skb))
+ return 0;
+
+ return vrf_prepare_mac_header(skb, vrf_dev, proto);
+}
+
static struct sk_buff *vrf_ip6_rcv(struct net_device *vrf_dev,
struct sk_buff *skb)
{
@@ -1335,9 +1390,15 @@ static struct sk_buff *vrf_ip6_rcv(struct net_device *vrf_dev,
skb->skb_iif = vrf_dev->ifindex;
if (!list_empty(&vrf_dev->ptype_all)) {
- skb_push(skb, skb->mac_len);
- dev_queue_xmit_nit(skb, vrf_dev);
- skb_pull(skb, skb->mac_len);
+ int err;
+
+ err = vrf_add_mac_header_if_unset(skb, vrf_dev,
+ ETH_P_IPV6);
+ if (likely(!err)) {
+ skb_push(skb, skb->mac_len);
+ dev_queue_xmit_nit(skb, vrf_dev);
+ skb_pull(skb, skb->mac_len);
+ }
}
IP6CB(skb)->flags |= IP6SKB_L3SLAVE;
@@ -1380,9 +1441,14 @@ static struct sk_buff *vrf_ip_rcv(struct net_device *vrf_dev,
vrf_rx_stats(vrf_dev, skb->len);
if (!list_empty(&vrf_dev->ptype_all)) {
- skb_push(skb, skb->mac_len);
- dev_queue_xmit_nit(skb, vrf_dev);
- skb_pull(skb, skb->mac_len);
+ int err;
+
+ err = vrf_add_mac_header_if_unset(skb, vrf_dev, ETH_P_IP);
+ if (likely(!err)) {
+ skb_push(skb, skb->mac_len);
+ dev_queue_xmit_nit(skb, vrf_dev);
+ skb_pull(skb, skb->mac_len);
+ }
}
skb = vrf_rcv_nfhook(NFPROTO_IPV4, NF_INET_PRE_ROUTING, skb, vrf_dev);
--
2.20.1
^ permalink raw reply related
* [net-next v3 7/8] selftests: add selftest for the SRv6 End.DT4 behavior
From: Andrea Mayer @ 2020-11-23 18:28 UTC (permalink / raw)
To: David S. Miller, David Ahern, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Jakub Kicinski, Shuah Khan, Shrijeet Mukherjee,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, netdev, linux-kernel, linux-kselftest
Cc: Nathan Chancellor, Stefano Salsano, Paolo Lungaroni,
Ahmed Abdelsalam, Andrea Mayer
In-Reply-To: <20201123182857.4640-1-andrea.mayer@uniroma2.it>
this selftest is designed for evaluating the new SRv6 End.DT4 behavior
used, in this example, for implementing IPv4 L3 VPN use cases.
Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
Reviewed-by: David Ahern <dsahern@kernel.org>
---
.../selftests/net/srv6_end_dt4_l3vpn_test.sh | 494 ++++++++++++++++++
1 file changed, 494 insertions(+)
create mode 100755 tools/testing/selftests/net/srv6_end_dt4_l3vpn_test.sh
diff --git a/tools/testing/selftests/net/srv6_end_dt4_l3vpn_test.sh b/tools/testing/selftests/net/srv6_end_dt4_l3vpn_test.sh
new file mode 100755
index 000000000000..ad7a9fc59934
--- /dev/null
+++ b/tools/testing/selftests/net/srv6_end_dt4_l3vpn_test.sh
@@ -0,0 +1,494 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# author: Andrea Mayer <andrea.mayer@uniroma2.it>
+
+# This test is designed for evaluating the new SRv6 End.DT4 behavior used for
+# implementing IPv4 L3 VPN use cases.
+#
+# Hereafter a network diagram is shown, where two different tenants (named 100
+# and 200) offer IPv4 L3 VPN services allowing hosts to communicate with each
+# other across an IPv6 network.
+#
+# Only hosts belonging to the same tenant (and to the same VPN) can communicate
+# with each other. Instead, the communication among hosts of different tenants
+# is forbidden.
+# In other words, hosts hs-t100-1 and hs-t100-2 are connected through the IPv4
+# L3 VPN of tenant 100 while hs-t200-3 and hs-t200-4 are connected using the
+# IPv4 L3 VPN of tenant 200. Cross connection between tenant 100 and tenant 200
+# is forbidden and thus, for example, hs-t100-1 cannot reach hs-t200-3 and vice
+# versa.
+#
+# Routers rt-1 and rt-2 implement IPv4 L3 VPN services leveraging the SRv6
+# architecture. The key components for such VPNs are: a) SRv6 Encap behavior,
+# b) SRv6 End.DT4 behavior and c) VRF.
+#
+# To explain how an IPv4 L3 VPN based on SRv6 works, let us briefly consider an
+# example where, within the same domain of tenant 100, the host hs-t100-1 pings
+# the host hs-t100-2.
+#
+# First of all, L2 reachability of the host hs-t100-2 is taken into account by
+# the router rt-1 which acts as an arp proxy.
+#
+# When the host hs-t100-1 sends an IPv4 packet destined to hs-t100-2, the
+# router rt-1 receives the packet on the internal veth-t100 interface. Such
+# interface is enslaved to the VRF vrf-100 whose associated table contains the
+# SRv6 Encap route for encapsulating any IPv4 packet in a IPv6 plus the Segment
+# Routing Header (SRH) packet. This packet is sent through the (IPv6) core
+# network up to the router rt-2 that receives it on veth0 interface.
+#
+# The rt-2 router uses the 'localsid' routing table to process incoming
+# IPv6+SRH packets which belong to the VPN of the tenant 100. For each of these
+# packets, the SRv6 End.DT4 behavior removes the outer IPv6+SRH headers and
+# performs the lookup on the vrf-100 table using the destination address of
+# the decapsulated IPv4 packet. Afterwards, the packet is sent to the host
+# hs-t100-2 through the veth-t100 interface.
+#
+# The ping response follows the same processing but this time the role of rt-1
+# and rt-2 are swapped.
+#
+# Of course, the IPv4 L3 VPN for tenant 200 works exactly as the IPv4 L3 VPN
+# for tenant 100. In this case, only hosts hs-t200-3 and hs-t200-4 are able to
+# connect with each other.
+#
+#
+# +-------------------+ +-------------------+
+# | | | |
+# | hs-t100-1 netns | | hs-t100-2 netns |
+# | | | |
+# | +-------------+ | | +-------------+ |
+# | | veth0 | | | | veth0 | |
+# | | 10.0.0.1/24 | | | | 10.0.0.2/24 | |
+# | +-------------+ | | +-------------+ |
+# | . | | . |
+# +-------------------+ +-------------------+
+# . .
+# . .
+# . .
+# +-----------------------------------+ +-----------------------------------+
+# | . | | . |
+# | +---------------+ | | +---------------- |
+# | | veth-t100 | | | | veth-t100 | |
+# | | 10.0.0.254/24 | +----------+ | | +----------+ | 10.0.0.254/24 | |
+# | +-------+-------+ | localsid | | | | localsid | +-------+-------- |
+# | | | table | | | | table | | |
+# | +----+----+ +----------+ | | +----------+ +----+----+ |
+# | | vrf-100 | | | | vrf-100 | |
+# | +---------+ +------------+ | | +------------+ +---------+ |
+# | | veth0 | | | | veth0 | |
+# | | fd00::1/64 |.|...|.| fd00::2/64 | |
+# | +---------+ +------------+ | | +------------+ +---------+ |
+# | | vrf-200 | | | | vrf-200 | |
+# | +----+----+ | | +----+----+ |
+# | | | | | |
+# | +-------+-------+ | | +-------+-------- |
+# | | veth-t200 | | | | veth-t200 | |
+# | | 10.0.0.254/24 | | | | 10.0.0.254/24 | |
+# | +---------------+ rt-1 netns | | rt-2 netns +---------------- |
+# | . | | . |
+# +-----------------------------------+ +-----------------------------------+
+# . .
+# . .
+# . .
+# . .
+# +-------------------+ +-------------------+
+# | . | | . |
+# | +-------------+ | | +-------------+ |
+# | | veth0 | | | | veth0 | |
+# | | 10.0.0.3/24 | | | | 10.0.0.4/24 | |
+# | +-------------+ | | +-------------+ |
+# | | | |
+# | hs-t200-3 netns | | hs-t200-4 netns |
+# | | | |
+# +-------------------+ +-------------------+
+#
+#
+# ~~~~~~~~~~~~~~~~~~~~~~~~~
+# | Network configuration |
+# ~~~~~~~~~~~~~~~~~~~~~~~~~
+#
+# rt-1: localsid table (table 90)
+# +-------------------------------------------------+
+# |SID |Action |
+# +-------------------------------------------------+
+# |fc00:21:100::6004|apply SRv6 End.DT4 vrftable 100|
+# +-------------------------------------------------+
+# |fc00:21:200::6004|apply SRv6 End.DT4 vrftable 200|
+# +-------------------------------------------------+
+#
+# rt-1: VRF tenant 100 (table 100)
+# +---------------------------------------------------+
+# |host |Action |
+# +---------------------------------------------------+
+# |10.0.0.2 |apply seg6 encap segs fc00:12:100::6004|
+# +---------------------------------------------------+
+# |10.0.0.0/24|forward to dev veth_t100 |
+# +---------------------------------------------------+
+#
+# rt-1: VRF tenant 200 (table 200)
+# +---------------------------------------------------+
+# |host |Action |
+# +---------------------------------------------------+
+# |10.0.0.4 |apply seg6 encap segs fc00:12:200::6004|
+# +---------------------------------------------------+
+# |10.0.0.0/24|forward to dev veth_t200 |
+# +---------------------------------------------------+
+#
+#
+# rt-2: localsid table (table 90)
+# +-------------------------------------------------+
+# |SID |Action |
+# +-------------------------------------------------+
+# |fc00:12:100::6004|apply SRv6 End.DT4 vrftable 100|
+# +-------------------------------------------------+
+# |fc00:12:200::6004|apply SRv6 End.DT4 vrftable 200|
+# +-------------------------------------------------+
+#
+# rt-2: VRF tenant 100 (table 100)
+# +---------------------------------------------------+
+# |host |Action |
+# +---------------------------------------------------+
+# |10.0.0.1 |apply seg6 encap segs fc00:21:100::6004|
+# +---------------------------------------------------+
+# |10.0.0.0/24|forward to dev veth_t100 |
+# +---------------------------------------------------+
+#
+# rt-2: VRF tenant 200 (table 200)
+# +---------------------------------------------------+
+# |host |Action |
+# +---------------------------------------------------+
+# |10.0.0.3 |apply seg6 encap segs fc00:21:200::6004|
+# +---------------------------------------------------+
+# |10.0.0.0/24|forward to dev veth_t200 |
+# +---------------------------------------------------+
+#
+
+readonly LOCALSID_TABLE_ID=90
+readonly IPv6_RT_NETWORK=fd00
+readonly IPv4_HS_NETWORK=10.0.0
+readonly VPN_LOCATOR_SERVICE=fc00
+PING_TIMEOUT_SEC=4
+
+ret=0
+
+PAUSE_ON_FAIL=${PAUSE_ON_FAIL:=no}
+
+log_test()
+{
+ local rc=$1
+ local expected=$2
+ local msg="$3"
+
+ if [ ${rc} -eq ${expected} ]; then
+ nsuccess=$((nsuccess+1))
+ printf "\n TEST: %-60s [ OK ]\n" "${msg}"
+ else
+ ret=1
+ nfail=$((nfail+1))
+ printf "\n TEST: %-60s [FAIL]\n" "${msg}"
+ if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
+ echo
+ echo "hit enter to continue, 'q' to quit"
+ read a
+ [ "$a" = "q" ] && exit 1
+ fi
+ fi
+}
+
+print_log_test_results()
+{
+ if [ "$TESTS" != "none" ]; then
+ printf "\nTests passed: %3d\n" ${nsuccess}
+ printf "Tests failed: %3d\n" ${nfail}
+ fi
+}
+
+log_section()
+{
+ echo
+ echo "################################################################################"
+ echo "TEST SECTION: $*"
+ echo "################################################################################"
+}
+
+cleanup()
+{
+ ip link del veth-rt-1 2>/dev/null || true
+ ip link del veth-rt-2 2>/dev/null || true
+
+ # destroy routers rt-* and hosts hs-*
+ for ns in $(ip netns show | grep -E 'rt-*|hs-*'); do
+ ip netns del ${ns} || true
+ done
+}
+
+# Setup the basic networking for the routers
+setup_rt_networking()
+{
+ local rt=$1
+ local nsname=rt-${rt}
+
+ ip netns add ${nsname}
+ ip link set veth-rt-${rt} netns ${nsname}
+ ip -netns ${nsname} link set veth-rt-${rt} name veth0
+
+ ip -netns ${nsname} addr add ${IPv6_RT_NETWORK}::${rt}/64 dev veth0
+ ip -netns ${nsname} link set veth0 up
+ ip -netns ${nsname} link set lo up
+
+ ip netns exec ${nsname} sysctl -wq net.ipv4.ip_forward=1
+ ip netns exec ${nsname} sysctl -wq net.ipv6.conf.all.forwarding=1
+}
+
+setup_hs()
+{
+ local hs=$1
+ local rt=$2
+ local tid=$3
+ local hsname=hs-t${tid}-${hs}
+ local rtname=rt-${rt}
+ local rtveth=veth-t${tid}
+
+ # set the networking for the host
+ ip netns add ${hsname}
+ ip -netns ${hsname} link add veth0 type veth peer name ${rtveth}
+ ip -netns ${hsname} link set ${rtveth} netns ${rtname}
+ ip -netns ${hsname} addr add ${IPv4_HS_NETWORK}.${hs}/24 dev veth0
+ ip -netns ${hsname} link set veth0 up
+ ip -netns ${hsname} link set lo up
+
+ # configure the VRF for the tenant X on the router which is directly
+ # connected to the source host.
+ ip -netns ${rtname} link add vrf-${tid} type vrf table ${tid}
+ ip -netns ${rtname} link set vrf-${tid} up
+
+ # enslave the veth-tX interface to the vrf-X in the access router
+ ip -netns ${rtname} link set ${rtveth} master vrf-${tid}
+ ip -netns ${rtname} addr add ${IPv4_HS_NETWORK}.254/24 dev ${rtveth}
+ ip -netns ${rtname} link set ${rtveth} up
+
+ ip netns exec ${rtname} sysctl -wq net.ipv4.conf.${rtveth}.proxy_arp=1
+
+ # disable the rp_filter otherwise the kernel gets confused about how
+ # to route decap ipv4 packets.
+ ip netns exec ${rtname} sysctl -wq net.ipv4.conf.all.rp_filter=0
+ ip netns exec ${rtname} sysctl -wq net.ipv4.conf.${rtveth}.rp_filter=0
+
+ ip netns exec ${rtname} sh -c "echo 1 > /proc/sys/net/vrf/strict_mode"
+}
+
+setup_vpn_config()
+{
+ local hssrc=$1
+ local rtsrc=$2
+ local hsdst=$3
+ local rtdst=$4
+ local tid=$5
+
+ local hssrc_name=hs-t${tid}-${hssrc}
+ local hsdst_name=hs-t${tid}-${hsdst}
+ local rtsrc_name=rt-${rtsrc}
+ local rtdst_name=rt-${rtdst}
+ local vpn_sid=${VPN_LOCATOR_SERVICE}:${hssrc}${hsdst}:${tid}::6004
+
+ # set the encap route for encapsulating packets which arrive from the
+ # host hssrc and destined to the access router rtsrc.
+ ip -netns ${rtsrc_name} -4 route add ${IPv4_HS_NETWORK}.${hsdst}/32 vrf vrf-${tid} \
+ encap seg6 mode encap segs ${vpn_sid} dev veth0
+ ip -netns ${rtsrc_name} -6 route add ${vpn_sid}/128 vrf vrf-${tid} \
+ via fd00::${rtdst} dev veth0
+
+ # set the decap route for decapsulating packets which arrive from
+ # the rtdst router and destined to the hsdst host.
+ ip -netns ${rtdst_name} -6 route add ${vpn_sid}/128 table ${LOCALSID_TABLE_ID} \
+ encap seg6local action End.DT4 vrftable ${tid} dev vrf-${tid}
+
+ # all sids for VPNs start with a common locator which is fc00::/16.
+ # Routes for handling the SRv6 End.DT4 behavior instances are grouped
+ # together in the 'localsid' table.
+ #
+ # NOTE: added only once
+ if [ -z "$(ip -netns ${rtdst_name} -6 rule show | \
+ grep "to ${VPN_LOCATOR_SERVICE}::/16 lookup ${LOCALSID_TABLE_ID}")" ]; then
+ ip -netns ${rtdst_name} -6 rule add \
+ to ${VPN_LOCATOR_SERVICE}::/16 \
+ lookup ${LOCALSID_TABLE_ID} prio 999
+ fi
+}
+
+setup()
+{
+ ip link add veth-rt-1 type veth peer name veth-rt-2
+ # setup the networking for router rt-1 and router rt-2
+ setup_rt_networking 1
+ setup_rt_networking 2
+
+ # setup two hosts for the tenant 100.
+ # - host hs-1 is directly connected to the router rt-1;
+ # - host hs-2 is directly connected to the router rt-2.
+ setup_hs 1 1 100 #args: host router tenant
+ setup_hs 2 2 100
+
+ # setup two hosts for the tenant 200
+ # - host hs-3 is directly connected to the router rt-1;
+ # - host hs-4 is directly connected to the router rt-2.
+ setup_hs 3 1 200
+ setup_hs 4 2 200
+
+ # setup the IPv4 L3 VPN which connects the host hs-t100-1 and host
+ # hs-t100-2 within the same tenant 100.
+ setup_vpn_config 1 1 2 2 100 #args: src_host src_router dst_host dst_router tenant
+ setup_vpn_config 2 2 1 1 100
+
+ # setup the IPv4 L3 VPN which connects the host hs-t200-3 and host
+ # hs-t200-4 within the same tenant 200.
+ setup_vpn_config 3 1 4 2 200
+ setup_vpn_config 4 2 3 1 200
+}
+
+check_rt_connectivity()
+{
+ local rtsrc=$1
+ local rtdst=$2
+
+ ip netns exec rt-${rtsrc} ping -c 1 -W 1 ${IPv6_RT_NETWORK}::${rtdst} \
+ >/dev/null 2>&1
+}
+
+check_and_log_rt_connectivity()
+{
+ local rtsrc=$1
+ local rtdst=$2
+
+ check_rt_connectivity ${rtsrc} ${rtdst}
+ log_test $? 0 "Routers connectivity: rt-${rtsrc} -> rt-${rtdst}"
+}
+
+check_hs_connectivity()
+{
+ local hssrc=$1
+ local hsdst=$2
+ local tid=$3
+
+ ip netns exec hs-t${tid}-${hssrc} ping -c 1 -W ${PING_TIMEOUT_SEC} \
+ ${IPv4_HS_NETWORK}.${hsdst} >/dev/null 2>&1
+}
+
+check_and_log_hs_connectivity()
+{
+ local hssrc=$1
+ local hsdst=$2
+ local tid=$3
+
+ check_hs_connectivity ${hssrc} ${hsdst} ${tid}
+ log_test $? 0 "Hosts connectivity: hs-t${tid}-${hssrc} -> hs-t${tid}-${hsdst} (tenant ${tid})"
+}
+
+check_and_log_hs_isolation()
+{
+ local hssrc=$1
+ local tidsrc=$2
+ local hsdst=$3
+ local tiddst=$4
+
+ check_hs_connectivity ${hssrc} ${hsdst} ${tidsrc}
+ # NOTE: ping should fail
+ log_test $? 1 "Hosts isolation: hs-t${tidsrc}-${hssrc} -X-> hs-t${tiddst}-${hsdst}"
+}
+
+
+check_and_log_hs2gw_connectivity()
+{
+ local hssrc=$1
+ local tid=$2
+
+ check_hs_connectivity ${hssrc} 254 ${tid}
+ log_test $? 0 "Hosts connectivity: hs-t${tid}-${hssrc} -> gw (tenant ${tid})"
+}
+
+router_tests()
+{
+ log_section "IPv6 routers connectivity test"
+
+ check_and_log_rt_connectivity 1 2
+ check_and_log_rt_connectivity 2 1
+}
+
+host2gateway_tests()
+{
+ log_section "IPv4 connectivity test among hosts and gateway"
+
+ check_and_log_hs2gw_connectivity 1 100
+ check_and_log_hs2gw_connectivity 2 100
+
+ check_and_log_hs2gw_connectivity 3 200
+ check_and_log_hs2gw_connectivity 4 200
+}
+
+host_vpn_tests()
+{
+ log_section "SRv6 VPN connectivity test among hosts in the same tenant"
+
+ check_and_log_hs_connectivity 1 2 100
+ check_and_log_hs_connectivity 2 1 100
+
+ check_and_log_hs_connectivity 3 4 200
+ check_and_log_hs_connectivity 4 3 200
+}
+
+host_vpn_isolation_tests()
+{
+ local i
+ local j
+ local k
+ local tmp
+ local l1="1 2"
+ local l2="3 4"
+ local t1=100
+ local t2=200
+
+ log_section "SRv6 VPN isolation test among hosts in different tentants"
+
+ for k in 0 1; do
+ for i in ${l1}; do
+ for j in ${l2}; do
+ check_and_log_hs_isolation ${i} ${t1} ${j} ${t2}
+ done
+ done
+
+ # let us test the reverse path
+ tmp="${l1}"; l1="${l2}"; l2="${tmp}"
+ tmp=${t1}; t1=${t2}; t2=${tmp}
+ done
+}
+
+if [ "$(id -u)" -ne 0 ];then
+ echo "SKIP: Need root privileges"
+ exit 0
+fi
+
+if [ ! -x "$(command -v ip)" ]; then
+ echo "SKIP: Could not run test without ip tool"
+ exit 0
+fi
+
+modprobe vrf &>/dev/null
+if [ ! -e /proc/sys/net/vrf/strict_mode ]; then
+ echo "SKIP: vrf sysctl does not exist"
+ exit 0
+fi
+
+cleanup &>/dev/null
+
+setup
+
+router_tests
+host2gateway_tests
+host_vpn_tests
+host_vpn_isolation_tests
+
+print_log_test_results
+
+cleanup &>/dev/null
+
+exit ${ret}
--
2.20.1
^ permalink raw reply related
* [net-next v3 6/8] seg6: add VRF support for SRv6 End.DT6 behavior
From: Andrea Mayer @ 2020-11-23 18:28 UTC (permalink / raw)
To: David S. Miller, David Ahern, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Jakub Kicinski, Shuah Khan, Shrijeet Mukherjee,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, netdev, linux-kernel, linux-kselftest
Cc: Nathan Chancellor, Stefano Salsano, Paolo Lungaroni,
Ahmed Abdelsalam, Andrea Mayer
In-Reply-To: <20201123182857.4640-1-andrea.mayer@uniroma2.it>
SRv6 End.DT6 is defined in the SRv6 Network Programming [1].
The Linux kernel already offers an implementation of the SRv6
End.DT6 behavior which permits IPv6 L3 VPNs over SRv6 networks. This
implementation is not particularly suitable in contexts where we need to
deploy IPv6 L3 VPNs among different tenants which share the same network
address schemes. The underlying problem lies in the fact that the
current version of DT6 (called legacy DT6 from now on) needs a complex
configuration to be applied on routers which requires ad-hoc routes and
routing policy rules to ensure the correct isolation of tenants.
Consequently, a new implementation of DT6 has been introduced with the
aim of simplifying the construction of IPv6 L3 VPN services in the
multi-tenant environment using SRv6 networks. To accomplish this task,
we reused the same VRF infrastructure and SRv6 core components already
exploited for implementing the SRv6 End.DT4 behavior.
Currently the two End.DT6 implementations coexist seamlessly and can be
used depending on the context and the user preferences. So, in order to
support both versions of DT6 a new attribute (vrftable) has been
introduced which allows us to differentiate the implementation of the
behavior to be used.
A SRv6 End.DT6 legacy behavior is still instantiated using a command
like the following one:
$ ip -6 route add 2001:db8::1 encap seg6local action End.DT6 table 100 dev eth0
While to instantiate the SRv6 End.DT6 in VRF mode, the command is still
pretty straight forward:
$ ip -6 route add 2001:db8::1 encap seg6local action End.DT6 vrftable 100 dev eth0.
Obviously as in the case of SRv6 End.DT4, the VRF strict_mode parameter
must be set (net.vrf.strict_mode=1) and the VRF associated with table
100 must exist.
Please note that the instances of SRv6 End.DT6 legacy and End.DT6 VRF
mode can coexist in the same system/configuration without problems.
[1] https://tools.ietf.org/html/draft-ietf-spring-srv6-network-programming
Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
---
net/ipv6/seg6_local.c | 76 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 76 insertions(+)
diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
index 117405985e6f..b257632f635e 100644
--- a/net/ipv6/seg6_local.c
+++ b/net/ipv6/seg6_local.c
@@ -497,6 +497,10 @@ static int __seg6_end_dt_vrf_build(struct seg6_local_lwt *slwt, const void *cfg,
info->proto = htons(ETH_P_IP);
info->hdrlen = sizeof(struct iphdr);
break;
+ case AF_INET6:
+ info->proto = htons(ETH_P_IPV6);
+ info->hdrlen = sizeof(struct ipv6hdr);
+ break;
default:
return -EINVAL;
}
@@ -649,6 +653,47 @@ static int seg6_end_dt4_build(struct seg6_local_lwt *slwt, const void *cfg,
{
return __seg6_end_dt_vrf_build(slwt, cfg, AF_INET, extack);
}
+
+static enum
+seg6_end_dt_mode seg6_end_dt6_parse_mode(struct seg6_local_lwt *slwt)
+{
+ unsigned long parsed_optattrs = slwt->parsed_optattrs;
+ bool legacy, vrfmode;
+
+ legacy = !!(parsed_optattrs & (1 << SEG6_LOCAL_TABLE));
+ vrfmode = !!(parsed_optattrs & (1 << SEG6_LOCAL_VRFTABLE));
+
+ if (!(legacy ^ vrfmode))
+ /* both are absent or present: invalid DT6 mode */
+ return DT_INVALID_MODE;
+
+ return legacy ? DT_LEGACY_MODE : DT_VRF_MODE;
+}
+
+static enum seg6_end_dt_mode seg6_end_dt6_get_mode(struct seg6_local_lwt *slwt)
+{
+ struct seg6_end_dt_info *info = &slwt->dt_info;
+
+ return info->mode;
+}
+
+static int seg6_end_dt6_build(struct seg6_local_lwt *slwt, const void *cfg,
+ struct netlink_ext_ack *extack)
+{
+ enum seg6_end_dt_mode mode = seg6_end_dt6_parse_mode(slwt);
+ struct seg6_end_dt_info *info = &slwt->dt_info;
+
+ switch (mode) {
+ case DT_LEGACY_MODE:
+ info->mode = DT_LEGACY_MODE;
+ return 0;
+ case DT_VRF_MODE:
+ return __seg6_end_dt_vrf_build(slwt, cfg, AF_INET6, extack);
+ default:
+ NL_SET_ERR_MSG(extack, "table or vrftable must be specified");
+ return -EINVAL;
+ }
+}
#endif
static int input_action_end_dt6(struct sk_buff *skb,
@@ -660,6 +705,28 @@ static int input_action_end_dt6(struct sk_buff *skb,
if (!pskb_may_pull(skb, sizeof(struct ipv6hdr)))
goto drop;
+#ifdef CONFIG_NET_L3_MASTER_DEV
+ if (seg6_end_dt6_get_mode(slwt) == DT_LEGACY_MODE)
+ goto legacy_mode;
+
+ /* DT6_VRF_MODE */
+ skb = end_dt_vrf_core(skb, slwt);
+ if (!skb)
+ /* packet has been processed and consumed by the VRF */
+ return 0;
+
+ if (IS_ERR(skb))
+ return PTR_ERR(skb);
+
+ /* note: this time we do not need to specify the table because the VRF
+ * takes care of selecting the correct table.
+ */
+ seg6_lookup_any_nexthop(skb, NULL, 0, true);
+
+ return dst_input(skb);
+
+legacy_mode:
+#endif
skb_set_transport_header(skb, sizeof(struct ipv6hdr));
seg6_lookup_any_nexthop(skb, NULL, slwt->table, true);
@@ -851,7 +918,16 @@ static struct seg6_action_desc seg6_action_table[] = {
},
{
.action = SEG6_LOCAL_ACTION_END_DT6,
+#ifdef CONFIG_NET_L3_MASTER_DEV
+ .attrs = 0,
+ .optattrs = (1 << SEG6_LOCAL_TABLE) |
+ (1 << SEG6_LOCAL_VRFTABLE),
+ .slwt_ops = {
+ .build_state = seg6_end_dt6_build,
+ },
+#else
.attrs = (1 << SEG6_LOCAL_TABLE),
+#endif
.input = input_action_end_dt6,
},
{
--
2.20.1
^ permalink raw reply related
* [net-next v3 3/8] seg6: add support for optional attributes in SRv6 behaviors
From: Andrea Mayer @ 2020-11-23 18:28 UTC (permalink / raw)
To: David S. Miller, David Ahern, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Jakub Kicinski, Shuah Khan, Shrijeet Mukherjee,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, netdev, linux-kernel, linux-kselftest
Cc: Nathan Chancellor, Stefano Salsano, Paolo Lungaroni,
Ahmed Abdelsalam, Andrea Mayer
In-Reply-To: <20201123182857.4640-1-andrea.mayer@uniroma2.it>
Before this patch, each SRv6 behavior specifies a set of required
attributes that must be provided by the userspace application when such
behavior is going to be instantiated. If at least one of the required
attributes is not provided, the creation of the behavior fails.
The SRv6 behavior framework lacks a way to manage optional attributes.
By definition, an optional attribute for a SRv6 behavior consists of an
attribute which may or may not be provided by the userspace. Therefore,
if an optional attribute is missing (and thus not supplied by the user)
the creation of the behavior goes ahead without any issue.
This patch explicitly differentiates the required attributes from the
optional attributes. In particular, each behavior can declare a set of
required attributes and a set of optional ones.
The semantic of the required attributes remains *totally* unaffected by
this patch. The introduction of the optional attributes does NOT impact
on the backward compatibility of the existing SRv6 behaviors.
It is essential to note that if an (optional or required) attribute is
supplied to a SRv6 behavior which does not expect it, the behavior
simply discards such attribute without generating any error or warning.
This operating mode remained unchanged both before and after the
introduction of the optional attributes extension.
The optional attributes are one of the key components used to implement
the SRv6 End.DT6 behavior based on the Virtual Routing and Forwarding
(VRF) framework. The optional attributes make possible the coexistence
of the already existing SRv6 End.DT6 implementation with the new SRv6
End.DT6 VRF-based implementation without breaking any backward
compatibility. Further details on the SRv6 End.DT6 behavior (VRF mode)
are reported in subsequent patches.
From the userspace point of view, the support for optional attributes DO
NOT require any changes to the userspace applications, i.e: iproute2
unless new attributes (required or optional) are needed.
Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
---
net/ipv6/seg6_local.c | 120 +++++++++++++++++++++++++++++++++++++-----
1 file changed, 106 insertions(+), 14 deletions(-)
diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
index aef39eab9be2..3b5657c622a0 100644
--- a/net/ipv6/seg6_local.c
+++ b/net/ipv6/seg6_local.c
@@ -36,6 +36,21 @@ struct seg6_local_lwt;
struct seg6_action_desc {
int action;
unsigned long attrs;
+
+ /* The optattrs field is used for specifying all the optional
+ * attributes supported by a specific behavior.
+ * It means that if one of these attributes is not provided in the
+ * netlink message during the behavior creation, no errors will be
+ * returned to the userspace.
+ *
+ * Each attribute can be only of two types (mutually exclusive):
+ * 1) required or 2) optional.
+ * Every user MUST obey to this rule! If you set an attribute as
+ * required the same attribute CANNOT be set as optional and vice
+ * versa.
+ */
+ unsigned long optattrs;
+
int (*input)(struct sk_buff *skb, struct seg6_local_lwt *slwt);
int static_headroom;
};
@@ -57,6 +72,10 @@ struct seg6_local_lwt {
int headroom;
struct seg6_action_desc *desc;
+ /* unlike the required attrs, we have to track the optional attributes
+ * that have been effectively parsed.
+ */
+ unsigned long parsed_optattrs;
};
static struct seg6_local_lwt *seg6_local_lwtunnel(struct lwtunnel_state *lwt)
@@ -959,26 +978,26 @@ static struct seg6_action_param seg6_action_params[SEG6_LOCAL_MAX + 1] = {
};
/* call the destroy() callback (if available) for each set attribute in
- * @slwt, starting from the first attribute up to the @max_parsed (excluded)
- * attribute.
+ * @parsed_attrs, starting from the first attribute up to the @max_parsed
+ * (excluded) attribute.
*/
-static void __destroy_attrs(int max_parsed, struct seg6_local_lwt *slwt)
+static void __destroy_attrs(unsigned long parsed_attrs, int max_parsed,
+ struct seg6_local_lwt *slwt)
{
- unsigned long attrs = slwt->desc->attrs;
struct seg6_action_param *param;
int i;
/* Every required seg6local attribute is identified by an ID which is
* encoded as a flag (i.e: 1 << ID) in the 'attrs' bitmask;
*
- * We scan the 'attrs' bitmask, starting from the first attribute
+ * We scan the 'parsed_attrs' bitmask, starting from the first attribute
* up to the @max_parsed (excluded) attribute.
* For each set attribute, we retrieve the corresponding destroy()
* callback. If the callback is not available, then we skip to the next
* attribute; otherwise, we call the destroy() callback.
*/
for (i = 0; i < max_parsed; ++i) {
- if (!(attrs & (1 << i)))
+ if (!(parsed_attrs & (1 << i)))
continue;
param = &seg6_action_params[i];
@@ -993,13 +1012,54 @@ static void __destroy_attrs(int max_parsed, struct seg6_local_lwt *slwt)
*/
static void destroy_attrs(struct seg6_local_lwt *slwt)
{
- __destroy_attrs(SEG6_LOCAL_MAX + 1, slwt);
+ unsigned long attrs = slwt->desc->attrs | slwt->parsed_optattrs;
+
+ __destroy_attrs(attrs, SEG6_LOCAL_MAX + 1, slwt);
+}
+
+static int parse_nla_optional_attrs(struct nlattr **attrs,
+ struct seg6_local_lwt *slwt)
+{
+ struct seg6_action_desc *desc = slwt->desc;
+ unsigned long parsed_optattrs = 0;
+ struct seg6_action_param *param;
+ int err, i;
+
+ for (i = 0; i < SEG6_LOCAL_MAX + 1; ++i) {
+ if (!(desc->optattrs & (1 << i)) || !attrs[i])
+ continue;
+
+ /* once here, the i-th attribute is provided by the
+ * userspace AND it is identified optional as well.
+ */
+ param = &seg6_action_params[i];
+
+ err = param->parse(attrs, slwt);
+ if (err < 0)
+ goto parse_optattrs_err;
+
+ /* current attribute has been correctly parsed */
+ parsed_optattrs |= (1 << i);
+ }
+
+ /* store in the tunnel state all the optional attributed successfully
+ * parsed.
+ */
+ slwt->parsed_optattrs = parsed_optattrs;
+
+ return 0;
+
+parse_optattrs_err:
+ __destroy_attrs(parsed_optattrs, i, slwt);
+
+ return err;
}
static int parse_nla_action(struct nlattr **attrs, struct seg6_local_lwt *slwt)
{
struct seg6_action_param *param;
struct seg6_action_desc *desc;
+ unsigned long invalid_attrs;
int i, err;
desc = __get_action_desc(slwt->action);
@@ -1012,6 +1072,26 @@ static int parse_nla_action(struct nlattr **attrs, struct seg6_local_lwt *slwt)
slwt->desc = desc;
slwt->headroom += desc->static_headroom;
+ /* Forcing the desc->optattrs *set* and the desc->attrs *set* to be
+ * disjoined, this allow us to release acquired resources by optional
+ * attributes and by required attributes independently from each other
+ * without any interfarence.
+ * In other terms, we are sure that we do not release some the acquired
+ * resources twice.
+ *
+ * Note that if an attribute is configured both as required and as
+ * optional, it means that the user has messed something up in the
+ * seg6_action_table. Therefore, this check is required for SRv6
+ * behaviors to work properly.
+ */
+ invalid_attrs = desc->attrs & desc->optattrs;
+ if (invalid_attrs) {
+ WARN_ONCE(1,
+ "An attribute cannot be both required AND optional");
+ return -EINVAL;
+ }
+
+ /* parse the required attributes */
for (i = 0; i < SEG6_LOCAL_MAX + 1; i++) {
if (desc->attrs & (1 << i)) {
if (!attrs[i])
@@ -1021,17 +1101,22 @@ static int parse_nla_action(struct nlattr **attrs, struct seg6_local_lwt *slwt)
err = param->parse(attrs, slwt);
if (err < 0)
- goto parse_err;
+ goto parse_attrs_err;
}
}
+ /* parse the optional attributes, if any */
+ err = parse_nla_optional_attrs(attrs, slwt);
+ if (err < 0)
+ goto parse_attrs_err;
+
return 0;
-parse_err:
+parse_attrs_err:
/* release any resource that may have been acquired during the i-1
* parse() operations.
*/
- __destroy_attrs(i, slwt);
+ __destroy_attrs(desc->attrs, i, slwt);
return err;
}
@@ -1096,13 +1181,16 @@ static int seg6_local_fill_encap(struct sk_buff *skb,
{
struct seg6_local_lwt *slwt = seg6_local_lwtunnel(lwt);
struct seg6_action_param *param;
+ unsigned long attrs;
int i, err;
if (nla_put_u32(skb, SEG6_LOCAL_ACTION, slwt->action))
return -EMSGSIZE;
+ attrs = slwt->desc->attrs | slwt->parsed_optattrs;
+
for (i = 0; i < SEG6_LOCAL_MAX + 1; i++) {
- if (slwt->desc->attrs & (1 << i)) {
+ if (attrs & (1 << i)) {
param = &seg6_action_params[i];
err = param->put(skb, slwt);
if (err < 0)
@@ -1121,7 +1209,7 @@ static int seg6_local_get_encap_size(struct lwtunnel_state *lwt)
nlsize = nla_total_size(4); /* action */
- attrs = slwt->desc->attrs;
+ attrs = slwt->desc->attrs | slwt->parsed_optattrs;
if (attrs & (1 << SEG6_LOCAL_SRH))
nlsize += nla_total_size((slwt->srh->hdrlen + 1) << 3);
@@ -1154,6 +1242,7 @@ static int seg6_local_cmp_encap(struct lwtunnel_state *a,
{
struct seg6_local_lwt *slwt_a, *slwt_b;
struct seg6_action_param *param;
+ unsigned long attrs_a, attrs_b;
int i;
slwt_a = seg6_local_lwtunnel(a);
@@ -1162,11 +1251,14 @@ static int seg6_local_cmp_encap(struct lwtunnel_state *a,
if (slwt_a->action != slwt_b->action)
return 1;
- if (slwt_a->desc->attrs != slwt_b->desc->attrs)
+ attrs_a = slwt_a->desc->attrs | slwt_a->parsed_optattrs;
+ attrs_b = slwt_b->desc->attrs | slwt_b->parsed_optattrs;
+
+ if (attrs_a != attrs_b)
return 1;
for (i = 0; i < SEG6_LOCAL_MAX + 1; i++) {
- if (slwt_a->desc->attrs & (1 << i)) {
+ if (attrs_a & (1 << i)) {
param = &seg6_action_params[i];
if (param->cmp(slwt_a, slwt_b))
return 1;
--
2.20.1
^ permalink raw reply related
* [net-next v3 2/8] seg6: improve management of behavior attributes
From: Andrea Mayer @ 2020-11-23 18:28 UTC (permalink / raw)
To: David S. Miller, David Ahern, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Jakub Kicinski, Shuah Khan, Shrijeet Mukherjee,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, netdev, linux-kernel, linux-kselftest
Cc: Nathan Chancellor, Stefano Salsano, Paolo Lungaroni,
Ahmed Abdelsalam, Andrea Mayer
In-Reply-To: <20201123182857.4640-1-andrea.mayer@uniroma2.it>
Depending on the attribute (i.e.: SEG6_LOCAL_SRH, SEG6_LOCAL_TABLE, etc),
the parse() callback performs some validity checks on the provided input
and updates the tunnel state (slwt) with the result of the parsing
operation. However, an attribute may also need to reserve some additional
resources (i.e.: memory or setting up an eBPF program) in the parse()
callback to complete the parsing operation.
The parse() callbacks are invoked by the parse_nla_action() for each
attribute belonging to a specific behavior. Given a behavior with N
attributes, if the parsing of the i-th attribute fails, the
parse_nla_action() returns immediately with an error. Nonetheless, the
resources acquired during the parsing of the i-1 attributes are not freed
by the parse_nla_action().
Attributes which acquire resources must release them *in an explicit way*
in both the seg6_local_{build/destroy}_state(). However, adding a new
attribute of this type requires changes to
seg6_local_{build/destroy}_state() to release the resources correctly.
The seg6local infrastructure still lacks a simple and structured way to
release the resources acquired in the parse() operations.
We introduced a new callback in the struct seg6_action_param named
destroy(). This callback releases any resource which may have been acquired
in the parse() counterpart. Each attribute may or may not implement the
destroy() callback depending on whether it needs to free some acquired
resources.
The destroy() callback comes with several of advantages:
1) we can have many attributes as we want for a given behavior with no
need to explicitly free the taken resources;
2) As in case of the seg6_local_build_state(), the
seg6_local_destroy_state() does not need to handle the release of
resources directly. Indeed, it calls the destroy_attrs() function which
is in charge of calling the destroy() callback for every set attribute.
We do not need to patch seg6_local_{build/destroy}_state() anymore as
we add new attributes;
3) the code is more readable and better structured. Indeed, all the
information needed to handle a given attribute are contained in only
one place;
4) it facilitates the integration with new features introduced in further
patches.
Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
---
net/ipv6/seg6_local.c | 80 +++++++++++++++++++++++++++++++++++++------
1 file changed, 70 insertions(+), 10 deletions(-)
diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
index eba23279912d..aef39eab9be2 100644
--- a/net/ipv6/seg6_local.c
+++ b/net/ipv6/seg6_local.c
@@ -710,6 +710,11 @@ static int cmp_nla_srh(struct seg6_local_lwt *a, struct seg6_local_lwt *b)
return memcmp(a->srh, b->srh, len);
}
+static void destroy_attr_srh(struct seg6_local_lwt *slwt)
+{
+ kfree(slwt->srh);
+}
+
static int parse_nla_table(struct nlattr **attrs, struct seg6_local_lwt *slwt)
{
slwt->table = nla_get_u32(attrs[SEG6_LOCAL_TABLE]);
@@ -901,16 +906,30 @@ static int cmp_nla_bpf(struct seg6_local_lwt *a, struct seg6_local_lwt *b)
return strcmp(a->bpf.name, b->bpf.name);
}
+static void destroy_attr_bpf(struct seg6_local_lwt *slwt)
+{
+ kfree(slwt->bpf.name);
+ if (slwt->bpf.prog)
+ bpf_prog_put(slwt->bpf.prog);
+}
+
struct seg6_action_param {
int (*parse)(struct nlattr **attrs, struct seg6_local_lwt *slwt);
int (*put)(struct sk_buff *skb, struct seg6_local_lwt *slwt);
int (*cmp)(struct seg6_local_lwt *a, struct seg6_local_lwt *b);
+
+ /* optional destroy() callback useful for releasing resources which
+ * have been previously acquired in the corresponding parse()
+ * function.
+ */
+ void (*destroy)(struct seg6_local_lwt *slwt);
};
static struct seg6_action_param seg6_action_params[SEG6_LOCAL_MAX + 1] = {
[SEG6_LOCAL_SRH] = { .parse = parse_nla_srh,
.put = put_nla_srh,
- .cmp = cmp_nla_srh },
+ .cmp = cmp_nla_srh,
+ .destroy = destroy_attr_srh },
[SEG6_LOCAL_TABLE] = { .parse = parse_nla_table,
.put = put_nla_table,
@@ -934,10 +953,49 @@ static struct seg6_action_param seg6_action_params[SEG6_LOCAL_MAX + 1] = {
[SEG6_LOCAL_BPF] = { .parse = parse_nla_bpf,
.put = put_nla_bpf,
- .cmp = cmp_nla_bpf },
+ .cmp = cmp_nla_bpf,
+ .destroy = destroy_attr_bpf },
};
+/* call the destroy() callback (if available) for each set attribute in
+ * @slwt, starting from the first attribute up to the @max_parsed (excluded)
+ * attribute.
+ */
+static void __destroy_attrs(int max_parsed, struct seg6_local_lwt *slwt)
+{
+ unsigned long attrs = slwt->desc->attrs;
+ struct seg6_action_param *param;
+ int i;
+
+ /* Every required seg6local attribute is identified by an ID which is
+ * encoded as a flag (i.e: 1 << ID) in the 'attrs' bitmask;
+ *
+ * We scan the 'attrs' bitmask, starting from the first attribute
+ * up to the @max_parsed (excluded) attribute.
+ * For each set attribute, we retrieve the corresponding destroy()
+ * callback. If the callback is not available, then we skip to the next
+ * attribute; otherwise, we call the destroy() callback.
+ */
+ for (i = 0; i < max_parsed; ++i) {
+ if (!(attrs & (1 << i)))
+ continue;
+
+ param = &seg6_action_params[i];
+
+ if (param->destroy)
+ param->destroy(slwt);
+ }
+}
+
+/* release all the resources that may have been acquired during parsing
+ * operations.
+ */
+static void destroy_attrs(struct seg6_local_lwt *slwt)
+{
+ __destroy_attrs(SEG6_LOCAL_MAX + 1, slwt);
+}
+
static int parse_nla_action(struct nlattr **attrs, struct seg6_local_lwt *slwt)
{
struct seg6_action_param *param;
@@ -963,11 +1021,19 @@ static int parse_nla_action(struct nlattr **attrs, struct seg6_local_lwt *slwt)
err = param->parse(attrs, slwt);
if (err < 0)
- return err;
+ goto parse_err;
}
}
return 0;
+
+parse_err:
+ /* release any resource that may have been acquired during the i-1
+ * parse() operations.
+ */
+ __destroy_attrs(i, slwt);
+
+ return err;
}
static int seg6_local_build_state(struct net *net, struct nlattr *nla,
@@ -1012,7 +1078,6 @@ static int seg6_local_build_state(struct net *net, struct nlattr *nla,
return 0;
out_free:
- kfree(slwt->srh);
kfree(newts);
return err;
}
@@ -1021,12 +1086,7 @@ static void seg6_local_destroy_state(struct lwtunnel_state *lwt)
{
struct seg6_local_lwt *slwt = seg6_local_lwtunnel(lwt);
- kfree(slwt->srh);
-
- if (slwt->desc->attrs & (1 << SEG6_LOCAL_BPF)) {
- kfree(slwt->bpf.name);
- bpf_prog_put(slwt->bpf.prog);
- }
+ destroy_attrs(slwt);
return;
}
--
2.20.1
^ permalink raw reply related
* [net-next v3 8/8] selftests: add selftest for the SRv6 End.DT6 (VRF) behavior
From: Andrea Mayer @ 2020-11-23 18:28 UTC (permalink / raw)
To: David S. Miller, David Ahern, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Jakub Kicinski, Shuah Khan, Shrijeet Mukherjee,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, netdev, linux-kernel, linux-kselftest
Cc: Nathan Chancellor, Stefano Salsano, Paolo Lungaroni,
Ahmed Abdelsalam, Andrea Mayer
In-Reply-To: <20201123182857.4640-1-andrea.mayer@uniroma2.it>
this selftest is designed for evaluating the new SRv6 End.DT6 (VRF) behavior
used, in this example, for implementing IPv6 L3 VPN use cases.
Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
Signed-off-by: Paolo Lungaroni <paolo.lungaroni@cnit.it>
---
.../selftests/net/srv6_end_dt6_l3vpn_test.sh | 502 ++++++++++++++++++
1 file changed, 502 insertions(+)
create mode 100755 tools/testing/selftests/net/srv6_end_dt6_l3vpn_test.sh
diff --git a/tools/testing/selftests/net/srv6_end_dt6_l3vpn_test.sh b/tools/testing/selftests/net/srv6_end_dt6_l3vpn_test.sh
new file mode 100755
index 000000000000..68708f5e26a0
--- /dev/null
+++ b/tools/testing/selftests/net/srv6_end_dt6_l3vpn_test.sh
@@ -0,0 +1,502 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# author: Andrea Mayer <andrea.mayer@uniroma2.it>
+# author: Paolo Lungaroni <paolo.lungaroni@cnit.it>
+
+# This test is designed for evaluating the new SRv6 End.DT6 behavior used for
+# implementing IPv6 L3 VPN use cases.
+#
+# Hereafter a network diagram is shown, where two different tenants (named 100
+# and 200) offer IPv6 L3 VPN services allowing hosts to communicate with each
+# other across an IPv6 network.
+#
+# Only hosts belonging to the same tenant (and to the same VPN) can communicate
+# with each other. Instead, the communication among hosts of different tenants
+# is forbidden.
+# In other words, hosts hs-t100-1 and hs-t100-2 are connected through the IPv6
+# L3 VPN of tenant 100 while hs-t200-3 and hs-t200-4 are connected using the
+# IPv6 L3 VPN of tenant 200. Cross connection between tenant 100 and tenant 200
+# is forbidden and thus, for example, hs-t100-1 cannot reach hs-t200-3 and vice
+# versa.
+#
+# Routers rt-1 and rt-2 implement IPv6 L3 VPN services leveraging the SRv6
+# architecture. The key components for such VPNs are: a) SRv6 Encap behavior,
+# b) SRv6 End.DT6 behavior and c) VRF.
+#
+# To explain how an IPv6 L3 VPN based on SRv6 works, let us briefly consider an
+# example where, within the same domain of tenant 100, the host hs-t100-1 pings
+# the host hs-t100-2.
+#
+# First of all, L2 reachability of the host hs-t100-2 is taken into account by
+# the router rt-1 which acts as a ndp proxy.
+#
+# When the host hs-t100-1 sends an IPv6 packet destined to hs-t100-2, the
+# router rt-1 receives the packet on the internal veth-t100 interface. Such
+# interface is enslaved to the VRF vrf-100 whose associated table contains the
+# SRv6 Encap route for encapsulating any IPv6 packet in a IPv6 plus the Segment
+# Routing Header (SRH) packet. This packet is sent through the (IPv6) core
+# network up to the router rt-2 that receives it on veth0 interface.
+#
+# The rt-2 router uses the 'localsid' routing table to process incoming
+# IPv6+SRH packets which belong to the VPN of the tenant 100. For each of these
+# packets, the SRv6 End.DT6 behavior removes the outer IPv6+SRH headers and
+# performs the lookup on the vrf-100 table using the destination address of
+# the decapsulated IPv6 packet. Afterwards, the packet is sent to the host
+# hs-t100-2 through the veth-t100 interface.
+#
+# The ping response follows the same processing but this time the role of rt-1
+# and rt-2 are swapped.
+#
+# Of course, the IPv6 L3 VPN for tenant 200 works exactly as the IPv6 L3 VPN
+# for tenant 100. In this case, only hosts hs-t200-3 and hs-t200-4 are able to
+# connect with each other.
+#
+#
+# +-------------------+ +-------------------+
+# | | | |
+# | hs-t100-1 netns | | hs-t100-2 netns |
+# | | | |
+# | +-------------+ | | +-------------+ |
+# | | veth0 | | | | veth0 | |
+# | | cafe::1/64 | | | | cafe::2/64 | |
+# | +-------------+ | | +-------------+ |
+# | . | | . |
+# +-------------------+ +-------------------+
+# . .
+# . .
+# . .
+# +-----------------------------------+ +-----------------------------------+
+# | . | | . |
+# | +---------------+ | | +---------------- |
+# | | veth-t100 | | | | veth-t100 | |
+# | | cafe::254/64 | +----------+ | | +----------+ | cafe::254/64 | |
+# | +-------+-------+ | localsid | | | | localsid | +-------+-------- |
+# | | | table | | | | table | | |
+# | +----+----+ +----------+ | | +----------+ +----+----+ |
+# | | vrf-100 | | | | vrf-100 | |
+# | +---------+ +------------+ | | +------------+ +---------+ |
+# | | veth0 | | | | veth0 | |
+# | | fd00::1/64 |.|...|.| fd00::2/64 | |
+# | +---------+ +------------+ | | +------------+ +---------+ |
+# | | vrf-200 | | | | vrf-200 | |
+# | +----+----+ | | +----+----+ |
+# | | | | | |
+# | +-------+-------+ | | +-------+-------- |
+# | | veth-t200 | | | | veth-t200 | |
+# | | cafe::254/64 | | | | cafe::254/64 | |
+# | +---------------+ rt-1 netns | | rt-2 netns +---------------- |
+# | . | | . |
+# +-----------------------------------+ +-----------------------------------+
+# . .
+# . .
+# . .
+# . .
+# +-------------------+ +-------------------+
+# | . | | . |
+# | +-------------+ | | +-------------+ |
+# | | veth0 | | | | veth0 | |
+# | | cafe::3/64 | | | | cafe::4/64 | |
+# | +-------------+ | | +-------------+ |
+# | | | |
+# | hs-t200-3 netns | | hs-t200-4 netns |
+# | | | |
+# +-------------------+ +-------------------+
+#
+#
+# ~~~~~~~~~~~~~~~~~~~~~~~~~
+# | Network configuration |
+# ~~~~~~~~~~~~~~~~~~~~~~~~~
+#
+# rt-1: localsid table (table 90)
+# +-------------------------------------------------+
+# |SID |Action |
+# +-------------------------------------------------+
+# |fc00:21:100::6006|apply SRv6 End.DT6 vrftable 100|
+# +-------------------------------------------------+
+# |fc00:21:200::6006|apply SRv6 End.DT6 vrftable 200|
+# +-------------------------------------------------+
+#
+# rt-1: VRF tenant 100 (table 100)
+# +---------------------------------------------------+
+# |host |Action |
+# +---------------------------------------------------+
+# |cafe::2 |apply seg6 encap segs fc00:12:100::6006|
+# +---------------------------------------------------+
+# |cafe::/64 |forward to dev veth_t100 |
+# +---------------------------------------------------+
+#
+# rt-1: VRF tenant 200 (table 200)
+# +---------------------------------------------------+
+# |host |Action |
+# +---------------------------------------------------+
+# |cafe::4 |apply seg6 encap segs fc00:12:200::6006|
+# +---------------------------------------------------+
+# |cafe::/64 |forward to dev veth_t200 |
+# +---------------------------------------------------+
+#
+#
+# rt-2: localsid table (table 90)
+# +-------------------------------------------------+
+# |SID |Action |
+# +-------------------------------------------------+
+# |fc00:12:100::6006|apply SRv6 End.DT6 vrftable 100|
+# +-------------------------------------------------+
+# |fc00:12:200::6006|apply SRv6 End.DT6 vrftable 200|
+# +-------------------------------------------------+
+#
+# rt-2: VRF tenant 100 (table 100)
+# +---------------------------------------------------+
+# |host |Action |
+# +---------------------------------------------------+
+# |cafe::1 |apply seg6 encap segs fc00:21:100::6006|
+# +---------------------------------------------------+
+# |cafe::/64 |forward to dev veth_t100 |
+# +---------------------------------------------------+
+#
+# rt-2: VRF tenant 200 (table 200)
+# +---------------------------------------------------+
+# |host |Action |
+# +---------------------------------------------------+
+# |cafe::3 |apply seg6 encap segs fc00:21:200::6006|
+# +---------------------------------------------------+
+# |cafe::/64 |forward to dev veth_t200 |
+# +---------------------------------------------------+
+#
+
+readonly LOCALSID_TABLE_ID=90
+readonly IPv6_RT_NETWORK=fd00
+readonly IPv6_HS_NETWORK=cafe
+readonly VPN_LOCATOR_SERVICE=fc00
+PING_TIMEOUT_SEC=4
+
+ret=0
+
+PAUSE_ON_FAIL=${PAUSE_ON_FAIL:=no}
+
+log_test()
+{
+ local rc=$1
+ local expected=$2
+ local msg="$3"
+
+ if [ ${rc} -eq ${expected} ]; then
+ nsuccess=$((nsuccess+1))
+ printf "\n TEST: %-60s [ OK ]\n" "${msg}"
+ else
+ ret=1
+ nfail=$((nfail+1))
+ printf "\n TEST: %-60s [FAIL]\n" "${msg}"
+ if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
+ echo
+ echo "hit enter to continue, 'q' to quit"
+ read a
+ [ "$a" = "q" ] && exit 1
+ fi
+ fi
+}
+
+print_log_test_results()
+{
+ if [ "$TESTS" != "none" ]; then
+ printf "\nTests passed: %3d\n" ${nsuccess}
+ printf "Tests failed: %3d\n" ${nfail}
+ fi
+}
+
+log_section()
+{
+ echo
+ echo "################################################################################"
+ echo "TEST SECTION: $*"
+ echo "################################################################################"
+}
+
+cleanup()
+{
+ ip link del veth-rt-1 2>/dev/null || true
+ ip link del veth-rt-2 2>/dev/null || true
+
+ # destroy routers rt-* and hosts hs-*
+ for ns in $(ip netns show | grep -E 'rt-*|hs-*'); do
+ ip netns del ${ns} || true
+ done
+}
+
+# Setup the basic networking for the routers
+setup_rt_networking()
+{
+ local rt=$1
+ local nsname=rt-${rt}
+
+ ip netns add ${nsname}
+ ip link set veth-rt-${rt} netns ${nsname}
+ ip -netns ${nsname} link set veth-rt-${rt} name veth0
+
+ ip netns exec ${nsname} sysctl -wq net.ipv6.conf.all.accept_dad=0
+ ip netns exec ${nsname} sysctl -wq net.ipv6.conf.default.accept_dad=0
+
+ ip -netns ${nsname} addr add ${IPv6_RT_NETWORK}::${rt}/64 dev veth0 nodad
+ ip -netns ${nsname} link set veth0 up
+ ip -netns ${nsname} link set lo up
+
+ ip netns exec ${nsname} sysctl -wq net.ipv6.conf.all.forwarding=1
+}
+
+setup_hs()
+{
+ local hs=$1
+ local rt=$2
+ local tid=$3
+ local hsname=hs-t${tid}-${hs}
+ local rtname=rt-${rt}
+ local rtveth=veth-t${tid}
+
+ # set the networking for the host
+ ip netns add ${hsname}
+
+ ip netns exec ${hsname} sysctl -wq net.ipv6.conf.all.accept_dad=0
+ ip netns exec ${hsname} sysctl -wq net.ipv6.conf.default.accept_dad=0
+
+ ip -netns ${hsname} link add veth0 type veth peer name ${rtveth}
+ ip -netns ${hsname} link set ${rtveth} netns ${rtname}
+ ip -netns ${hsname} addr add ${IPv6_HS_NETWORK}::${hs}/64 dev veth0 nodad
+ ip -netns ${hsname} link set veth0 up
+ ip -netns ${hsname} link set lo up
+
+ # configure the VRF for the tenant X on the router which is directly
+ # connected to the source host.
+ ip -netns ${rtname} link add vrf-${tid} type vrf table ${tid}
+ ip -netns ${rtname} link set vrf-${tid} up
+
+ ip netns exec ${rtname} sysctl -wq net.ipv6.conf.all.accept_dad=0
+ ip netns exec ${rtname} sysctl -wq net.ipv6.conf.default.accept_dad=0
+
+ # enslave the veth-tX interface to the vrf-X in the access router
+ ip -netns ${rtname} link set ${rtveth} master vrf-${tid}
+ ip -netns ${rtname} addr add ${IPv6_HS_NETWORK}::254/64 dev ${rtveth} nodad
+ ip -netns ${rtname} link set ${rtveth} up
+
+ ip netns exec ${rtname} sysctl -wq net.ipv6.conf.${rtveth}.proxy_ndp=1
+
+ ip netns exec ${rtname} sh -c "echo 1 > /proc/sys/net/vrf/strict_mode"
+}
+
+setup_vpn_config()
+{
+ local hssrc=$1
+ local rtsrc=$2
+ local hsdst=$3
+ local rtdst=$4
+ local tid=$5
+
+ local hssrc_name=hs-t${tid}-${hssrc}
+ local hsdst_name=hs-t${tid}-${hsdst}
+ local rtsrc_name=rt-${rtsrc}
+ local rtdst_name=rt-${rtdst}
+ local rtveth=veth-t${tid}
+ local vpn_sid=${VPN_LOCATOR_SERVICE}:${hssrc}${hsdst}:${tid}::6006
+
+ ip -netns ${rtsrc_name} -6 neigh add proxy ${IPv6_HS_NETWORK}::${hsdst} dev ${rtveth}
+
+ # set the encap route for encapsulating packets which arrive from the
+ # host hssrc and destined to the access router rtsrc.
+ ip -netns ${rtsrc_name} -6 route add ${IPv6_HS_NETWORK}::${hsdst}/128 vrf vrf-${tid} \
+ encap seg6 mode encap segs ${vpn_sid} dev veth0
+ ip -netns ${rtsrc_name} -6 route add ${vpn_sid}/128 vrf vrf-${tid} \
+ via fd00::${rtdst} dev veth0
+
+ # set the decap route for decapsulating packets which arrive from
+ # the rtdst router and destined to the hsdst host.
+ ip -netns ${rtdst_name} -6 route add ${vpn_sid}/128 table ${LOCALSID_TABLE_ID} \
+ encap seg6local action End.DT6 vrftable ${tid} dev vrf-${tid}
+
+ # all sids for VPNs start with a common locator which is fc00::/16.
+ # Routes for handling the SRv6 End.DT6 behavior instances are grouped
+ # together in the 'localsid' table.
+ #
+ # NOTE: added only once
+ if [ -z "$(ip -netns ${rtdst_name} -6 rule show | \
+ grep "to ${VPN_LOCATOR_SERVICE}::/16 lookup ${LOCALSID_TABLE_ID}")" ]; then
+ ip -netns ${rtdst_name} -6 rule add \
+ to ${VPN_LOCATOR_SERVICE}::/16 \
+ lookup ${LOCALSID_TABLE_ID} prio 999
+ fi
+}
+
+setup()
+{
+ ip link add veth-rt-1 type veth peer name veth-rt-2
+ # setup the networking for router rt-1 and router rt-2
+ setup_rt_networking 1
+ setup_rt_networking 2
+
+ # setup two hosts for the tenant 100.
+ # - host hs-1 is directly connected to the router rt-1;
+ # - host hs-2 is directly connected to the router rt-2.
+ setup_hs 1 1 100 #args: host router tenant
+ setup_hs 2 2 100
+
+ # setup two hosts for the tenant 200
+ # - host hs-3 is directly connected to the router rt-1;
+ # - host hs-4 is directly connected to the router rt-2.
+ setup_hs 3 1 200
+ setup_hs 4 2 200
+
+ # setup the IPv6 L3 VPN which connects the host hs-t100-1 and host
+ # hs-t100-2 within the same tenant 100.
+ setup_vpn_config 1 1 2 2 100 #args: src_host src_router dst_host dst_router tenant
+ setup_vpn_config 2 2 1 1 100
+
+ # setup the IPv6 L3 VPN which connects the host hs-t200-3 and host
+ # hs-t200-4 within the same tenant 200.
+ setup_vpn_config 3 1 4 2 200
+ setup_vpn_config 4 2 3 1 200
+}
+
+check_rt_connectivity()
+{
+ local rtsrc=$1
+ local rtdst=$2
+
+ ip netns exec rt-${rtsrc} ping -c 1 -W 1 ${IPv6_RT_NETWORK}::${rtdst} \
+ >/dev/null 2>&1
+}
+
+check_and_log_rt_connectivity()
+{
+ local rtsrc=$1
+ local rtdst=$2
+
+ check_rt_connectivity ${rtsrc} ${rtdst}
+ log_test $? 0 "Routers connectivity: rt-${rtsrc} -> rt-${rtdst}"
+}
+
+check_hs_connectivity()
+{
+ local hssrc=$1
+ local hsdst=$2
+ local tid=$3
+
+ ip netns exec hs-t${tid}-${hssrc} ping -c 1 -W ${PING_TIMEOUT_SEC} \
+ ${IPv6_HS_NETWORK}::${hsdst} >/dev/null 2>&1
+}
+
+check_and_log_hs_connectivity()
+{
+ local hssrc=$1
+ local hsdst=$2
+ local tid=$3
+
+ check_hs_connectivity ${hssrc} ${hsdst} ${tid}
+ log_test $? 0 "Hosts connectivity: hs-t${tid}-${hssrc} -> hs-t${tid}-${hsdst} (tenant ${tid})"
+}
+
+check_and_log_hs_isolation()
+{
+ local hssrc=$1
+ local tidsrc=$2
+ local hsdst=$3
+ local tiddst=$4
+
+ check_hs_connectivity ${hssrc} ${hsdst} ${tidsrc}
+ # NOTE: ping should fail
+ log_test $? 1 "Hosts isolation: hs-t${tidsrc}-${hssrc} -X-> hs-t${tiddst}-${hsdst}"
+}
+
+
+check_and_log_hs2gw_connectivity()
+{
+ local hssrc=$1
+ local tid=$2
+
+ check_hs_connectivity ${hssrc} 254 ${tid}
+ log_test $? 0 "Hosts connectivity: hs-t${tid}-${hssrc} -> gw (tenant ${tid})"
+}
+
+router_tests()
+{
+ log_section "IPv6 routers connectivity test"
+
+ check_and_log_rt_connectivity 1 2
+ check_and_log_rt_connectivity 2 1
+}
+
+host2gateway_tests()
+{
+ log_section "IPv6 connectivity test among hosts and gateway"
+
+ check_and_log_hs2gw_connectivity 1 100
+ check_and_log_hs2gw_connectivity 2 100
+
+ check_and_log_hs2gw_connectivity 3 200
+ check_and_log_hs2gw_connectivity 4 200
+}
+
+host_vpn_tests()
+{
+ log_section "SRv6 VPN connectivity test among hosts in the same tenant"
+
+ check_and_log_hs_connectivity 1 2 100
+ check_and_log_hs_connectivity 2 1 100
+
+ check_and_log_hs_connectivity 3 4 200
+ check_and_log_hs_connectivity 4 3 200
+}
+
+host_vpn_isolation_tests()
+{
+ local i
+ local j
+ local k
+ local tmp
+ local l1="1 2"
+ local l2="3 4"
+ local t1=100
+ local t2=200
+
+ log_section "SRv6 VPN isolation test among hosts in different tentants"
+
+ for k in 0 1; do
+ for i in ${l1}; do
+ for j in ${l2}; do
+ check_and_log_hs_isolation ${i} ${t1} ${j} ${t2}
+ done
+ done
+
+ # let us test the reverse path
+ tmp="${l1}"; l1="${l2}"; l2="${tmp}"
+ tmp=${t1}; t1=${t2}; t2=${tmp}
+ done
+}
+
+if [ "$(id -u)" -ne 0 ];then
+ echo "SKIP: Need root privileges"
+ exit 0
+fi
+
+if [ ! -x "$(command -v ip)" ]; then
+ echo "SKIP: Could not run test without ip tool"
+ exit 0
+fi
+
+modprobe vrf &>/dev/null
+if [ ! -e /proc/sys/net/vrf/strict_mode ]; then
+ echo "SKIP: vrf sysctl does not exist"
+ exit 0
+fi
+
+cleanup &>/dev/null
+
+setup
+
+router_tests
+host2gateway_tests
+host_vpn_tests
+host_vpn_isolation_tests
+
+print_log_test_results
+
+cleanup &>/dev/null
+
+exit ${ret}
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net-next] netfilter: bridge: reset skb->pkt_type after NF_INET_POST_ROUTING traversal
From: Florian Westphal @ 2020-11-23 18:32 UTC (permalink / raw)
To: Antoine Tenart
Cc: kuba, pablo, kadlec, fw, roopa, nikolay, netdev, netfilter-devel,
coreteam, sbrivio
In-Reply-To: <20201123174902.622102-1-atenart@kernel.org>
Antoine Tenart <atenart@kernel.org> wrote:
> Netfilter changes PACKET_OTHERHOST to PACKET_HOST before invoking the
> hooks as, while it's an expected value for a bridge, routing expects
> PACKET_HOST. The change is undone later on after hook traversal. This
> can be seen with pairs of functions updating skb>pkt_type and then
> reverting it to its original value:
>
> For hook NF_INET_PRE_ROUTING:
> setup_pre_routing / br_nf_pre_routing_finish
>
> For hook NF_INET_FORWARD:
> br_nf_forward_ip / br_nf_forward_finish
>
> But the third case where netfilter does this, for hook
> NF_INET_POST_ROUTING, the packet type is changed in br_nf_post_routing
> but never reverted. A comment says:
>
> /* We assume any code from br_dev_queue_push_xmit onwards doesn't care
> * about the value of skb->pkt_type. */
[..]
> But when having a tunnel (say vxlan) attached to a bridge we have the
> following call trace:
> In this specific case, this creates issues such as when an ICMPv6 PTB
> should be sent back. When CONFIG_BRIDGE_NETFILTER is enabled, the PTB
> isn't sent (as skb_tunnel_check_pmtu checks if pkt_type is PACKET_HOST
> and returns early).
>
> If the comment is right and no one cares about the value of
> skb->pkt_type after br_dev_queue_push_xmit (which isn't true), resetting
> it to its original value should be safe.
That comment is 18 years old, safe bet noone thought of
ipv6-in-tunnel-interface-added-as-bridge-port back then.
Reviewed-by: Florian Westphal <fw@strlen.de>
^ permalink raw reply
* Re: [PATCH net-next v5] net/tun: Call netdev notifiers
From: Jakub Kicinski @ 2020-11-23 18:40 UTC (permalink / raw)
To: Martin Schiller; +Cc: davem, netdev, linux-kernel
In-Reply-To: <a00d2725bce23f451cd030b9e621a764@dev.tdt.de>
On Mon, 23 Nov 2020 07:18:07 +0100 Martin Schiller wrote:
> On 2020-11-20 19:28, Jakub Kicinski wrote:
> > On Wed, 18 Nov 2020 07:39:19 +0100 Martin Schiller wrote:
> >> Call netdev notifiers before and after changing the device type.
> >>
> >> Signed-off-by: Martin Schiller <ms@dev.tdt.de>
> >
> > This is a fix, right? Can you give an example of something that goes
> > wrong without this patch?
>
> This change is related to my latest patches to the X.25 Subsystem:
> https://patchwork.kernel.org/project/netdevbpf/list/?series=388087
>
> I use a tun interface in a XoT (X.25 over TCP) application and use the
> TUNSETLINK ioctl to change the device type to ARPHRD_X25.
> As the default device type is ARPHRD_NONE the initial NETDEV_REGISTER
> event won't be catched by the X.25 Stack.
>
> Therefore I have to use the NETDEV_POST_TYPE_CHANGE to make sure that
> the corresponding neighbour structure is created.
>
> I could imagine that other protocols have similar requirements.
>
> Whether this is a fix or a functional extension is hard to say.
>
> Some time ago there was also a corresponding patch for the WAN/HDLC
> subsystem:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=2f8364a291e8
Thanks for this info, applied to net-next.
^ permalink raw reply
* Re: [PATCH v8] tcp: fix race condition when creating child sockets from syncookies
From: Eric Dumazet @ 2020-11-23 18:43 UTC (permalink / raw)
To: Ricardo Dias, davem, kuba, kuznet, yoshfuji, edumazet
Cc: netdev, linux-kernel
In-Reply-To: <20201120111133.GA67501@rdias-suse-pc.lan>
On 11/20/20 12:11 PM, Ricardo Dias wrote:
> When the TCP stack is in SYN flood mode, the server child socket is
> created from the SYN cookie received in a TCP packet with the ACK flag
> set.
>
> The child socket is created when the server receives the first TCP
> packet with a valid SYN cookie from the client. Usually, this packet
> corresponds to the final step of the TCP 3-way handshake, the ACK
> packet. But is also possible to receive a valid SYN cookie from the
> first TCP data packet sent by the client, and thus create a child socket
> from that SYN cookie.
>
> Since a client socket is ready to send data as soon as it receives the
> SYN+ACK packet from the server, the client can send the ACK packet (sent
> by the TCP stack code), and the first data packet (sent by the userspace
> program) almost at the same time, and thus the server will equally
> receive the two TCP packets with valid SYN cookies almost at the same
> instant.
>
> When such event happens, the TCP stack code has a race condition that
> occurs between the momement a lookup is done to the established
> connections hashtable to check for the existence of a connection for the
> same client, and the moment that the child socket is added to the
> established connections hashtable. As a consequence, this race condition
> can lead to a situation where we add two child sockets to the
> established connections hashtable and deliver two sockets to the
> userspace program to the same client.
>
> This patch fixes the race condition by checking if an existing child
> socket exists for the same client when we are adding the second child
> socket to the established connections socket. If an existing child
> socket exists, we drop the packet and discard the second child socket
> to the same client.
>
> Signed-off-by: Ricardo Dias <rdias@singlestore.com>
Ok, lets keep this version, thanks !
Signed-off-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: netconsole deadlock with virtnet
From: Jakub Kicinski @ 2020-11-23 18:52 UTC (permalink / raw)
To: Steven Rostedt
Cc: Leon Romanovsky, Jason Wang, Sergey Senozhatsky,
Michael S. Tsirkin, Petr Mladek, John Ogness, virtualization,
Amit Shah, Itay Aveksis, Ran Rozenstein, netdev
In-Reply-To: <20201123093128.701cf81b@gandalf.local.home>
On Mon, 23 Nov 2020 09:31:28 -0500 Steven Rostedt wrote:
> On Mon, 23 Nov 2020 13:08:55 +0200
> Leon Romanovsky <leon@kernel.org> wrote:
>
>
> > [ 10.028024] Chain exists of:
> > [ 10.028025] console_owner --> target_list_lock --> _xmit_ETHER#2
>
> Note, the problem is that we have a location that grabs the xmit_lock while
> holding target_list_lock (and possibly console_owner).
Well, it try_locks the xmit_lock. Does lockdep understand try-locks?
(not that I condone the shenanigans that are going on here)
^ permalink raw reply
* Re: [PATCH 000/141] Fix fall-through warnings for Clang
From: Miguel Ojeda @ 2020-11-23 18:56 UTC (permalink / raw)
To: James Bottomley
Cc: Kees Cook, Jakub Kicinski, Gustavo A. R. Silva, linux-kernel,
alsa-devel, amd-gfx, bridge, ceph-devel, cluster-devel, coreteam,
devel, dm-devel, drbd-dev, dri-devel, GR-everest-linux-l2,
GR-Linux-NIC-Dev, intel-gfx, intel-wired-lan, keyrings,
linux1394-devel, linux-acpi, linux-afs, Linux ARM, linux-arm-msm,
linux-atm-general, linux-block, linux-can, linux-cifs,
Linux Crypto Mailing List, linux-decnet-user,
Ext4 Developers List, linux-fbdev, linux-geode, linux-gpio,
linux-hams, linux-hwmon, linux-i3c, linux-ide, linux-iio,
linux-input, linux-integrity, linux-mediatek,
Linux Media Mailing List, linux-mmc, Linux-MM, linux-mtd,
linux-nfs, linux-rdma, linux-renesas-soc, linux-scsi, linux-sctp,
linux-security-module, linux-stm32, linux-usb, linux-watchdog,
linux-wireless, Network Development, netfilter-devel, nouveau,
op-tee, oss-drivers, patches, rds-devel, reiserfs-devel,
samba-technical, selinux, target-devel, tipc-discussion,
usb-storage, virtualization, wcn36xx,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), xen-devel,
linux-hardening, Nick Desaulniers, Nathan Chancellor,
Miguel Ojeda, Joe Perches
In-Reply-To: <fc45750b6d0277c401015b7aa11e16cd15f32ab2.camel@HansenPartnership.com>
On Mon, Nov 23, 2020 at 4:58 PM James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
>
> Well, I used git. It says that as of today in Linus' tree we have 889
> patches related to fall throughs and the first series went in in
> october 2017 ... ignoring a couple of outliers back to February.
I can see ~10k insertions over ~1k commits and 15 years that mention a
fallthrough in the entire repo. That is including some commits (like
the biggest one, 960 insertions) that have nothing to do with C
fallthrough. A single kernel release has an order of magnitude more
changes than this...
But if we do the math, for an author, at even 1 minute per line change
and assuming nothing can be automated at all, it would take 1 month of
work. For maintainers, a couple of trivial lines is noise compared to
many other patches.
In fact, this discussion probably took more time than the time it
would take to review the 200 lines. :-)
> We're also complaining about the inability to recruit maintainers:
>
> https://www.theregister.com/2020/06/30/hard_to_find_linux_maintainers_says_torvalds/
>
> And burn out:
>
> http://antirez.com/news/129
Accepting trivial and useful 1-line patches is not what makes a
voluntary maintainer quit... Thankless work with demanding deadlines is.
> The whole crux of your argument seems to be maintainers' time isn't
> important so we should accept all trivial patches
I have not said that, at all. In fact, I am a voluntary one and I
welcome patches like this. It takes very little effort on my side to
review and it helps the kernel overall. Paid maintainers are the ones
that can take care of big features/reviews.
> What I'm actually trying to articulate is a way of measuring value of
> the patch vs cost ... it has nothing really to do with who foots the
> actual bill.
I understand your point, but you were the one putting it in terms of a
junior FTE. In my view, 1 month-work (worst case) is very much worth
removing a class of errors from a critical codebase.
> One thesis I'm actually starting to formulate is that this continual
> devaluing of maintainers is why we have so much difficulty keeping and
> recruiting them.
That may very well be true, but I don't feel anybody has devalued
maintainers in this discussion.
Cheers,
Miguel
^ permalink raw reply
* Re: [PATCH net-next v3 1/5] net: implement threaded-able napi poll loop support
From: Jakub Kicinski @ 2020-11-23 18:56 UTC (permalink / raw)
To: Wei Wang
Cc: David Miller, Linux Kernel Network Developers, Eric Dumazet,
Felix Fietkau, Paolo Abeni, Hannes Frederic Sowa, Hillf Danton
In-Reply-To: <CAEA6p_ATLr-=xQ8cZLJE3cbWn=cFx11kpWm0cV2J2hiaOVFPzg@mail.gmail.com>
On Sat, 21 Nov 2020 18:23:33 -0800 Wei Wang wrote:
> On Sat, Nov 21, 2020 at 4:31 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > On Wed, 18 Nov 2020 11:10:05 -0800 Wei Wang wrote:
> > > +int napi_set_threaded(struct napi_struct *n, bool threaded)
> > > +{
> > > + ASSERT_RTNL();
> > > +
> > > + if (n->dev->flags & IFF_UP)
> > > + return -EBUSY;
> > > +
> > > + if (threaded == !!test_bit(NAPI_STATE_THREADED, &n->state))
> > > + return 0;
> > > + if (threaded)
> > > + set_bit(NAPI_STATE_THREADED, &n->state);
> > > + else
> > > + clear_bit(NAPI_STATE_THREADED, &n->state);
> >
> > Do we really need the per-NAPI control here? Does anyone have use cases
> > where that makes sense? The user would be guessing which NAPI means
> > which queue and which bit, currently.
>
> Thanks for reviewing this.
> I think one use case might be that if the driver uses separate napi
> for tx and rx, one might want to only enable threaded mode for rx, and
> leave tx completion in interrupt mode.
Okay, but with separate IRQs/NAPIs that's really a guessing game in
terms of NAPI -> bit position. I'd rather we held off on the per-NAPI
control.
If anyone has a strong use for it now, please let us know.
^ permalink raw reply
* Re: [PATCH net-next v3 1/5] net: implement threaded-able napi poll loop support
From: Wei Wang @ 2020-11-23 19:07 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David Miller, Linux Kernel Network Developers, Eric Dumazet,
Felix Fietkau, Paolo Abeni, Hannes Frederic Sowa, Hillf Danton
In-Reply-To: <20201123105647.3ae683ed@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
On Mon, Nov 23, 2020 at 10:56 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Sat, 21 Nov 2020 18:23:33 -0800 Wei Wang wrote:
> > On Sat, Nov 21, 2020 at 4:31 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > > On Wed, 18 Nov 2020 11:10:05 -0800 Wei Wang wrote:
> > > > +int napi_set_threaded(struct napi_struct *n, bool threaded)
> > > > +{
> > > > + ASSERT_RTNL();
> > > > +
> > > > + if (n->dev->flags & IFF_UP)
> > > > + return -EBUSY;
> > > > +
> > > > + if (threaded == !!test_bit(NAPI_STATE_THREADED, &n->state))
> > > > + return 0;
> > > > + if (threaded)
> > > > + set_bit(NAPI_STATE_THREADED, &n->state);
> > > > + else
> > > > + clear_bit(NAPI_STATE_THREADED, &n->state);
> > >
> > > Do we really need the per-NAPI control here? Does anyone have use cases
> > > where that makes sense? The user would be guessing which NAPI means
> > > which queue and which bit, currently.
> >
> > Thanks for reviewing this.
> > I think one use case might be that if the driver uses separate napi
> > for tx and rx, one might want to only enable threaded mode for rx, and
> > leave tx completion in interrupt mode.
>
> Okay, but with separate IRQs/NAPIs that's really a guessing game in
> terms of NAPI -> bit position. I'd rather we held off on the per-NAPI
> control.
>
Yes. That is true. The bit position is dependent on the driver implementation.
> If anyone has a strong use for it now, please let us know.
OK. Will change it to per dev control if no one objects.
^ permalink raw reply
* Re: netconsole deadlock with virtnet
From: Steven Rostedt @ 2020-11-23 19:09 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Leon Romanovsky, Jason Wang, Sergey Senozhatsky,
Michael S. Tsirkin, Petr Mladek, John Ogness, virtualization,
Amit Shah, Itay Aveksis, Ran Rozenstein, netdev
In-Reply-To: <20201123105252.1c295138@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
On Mon, 23 Nov 2020 10:52:52 -0800
Jakub Kicinski <kuba@kernel.org> wrote:
> On Mon, 23 Nov 2020 09:31:28 -0500 Steven Rostedt wrote:
> > On Mon, 23 Nov 2020 13:08:55 +0200
> > Leon Romanovsky <leon@kernel.org> wrote:
> >
> >
> > > [ 10.028024] Chain exists of:
> > > [ 10.028025] console_owner --> target_list_lock --> _xmit_ETHER#2
> >
> > Note, the problem is that we have a location that grabs the xmit_lock while
> > holding target_list_lock (and possibly console_owner).
>
> Well, it try_locks the xmit_lock. Does lockdep understand try-locks?
>
> (not that I condone the shenanigans that are going on here)
Does it?
virtnet_poll_tx() {
__netif_tx_lock() {
spin_lock(&txq->_xmit_lock);
That looks like we can have:
CPU0 CPU1
---- ----
lock(xmit_lock)
lock(console)
lock(target_list_lock)
__netif_tx_lock()
lock(xmit_lock);
[BLOCKED]
<interrupt>
lock(console)
[BLOCKED]
DEADLOCK.
So where is the trylock here?
Perhaps you need the trylock in virtnet_poll_tx()?
-- Steve
^ permalink raw reply
* [PATCH V3 net 1/3] net: ena: handle bad request id in ena_netdev
From: Shay Agroskin @ 2020-11-23 19:08 UTC (permalink / raw)
To: kuba, netdev
Cc: Shay Agroskin, dwmw, zorik, matua, saeedb, msw, aliguori, nafea,
gtzalik, netanel, alisaidi, benh, akiyano, sameehj, ndagan,
Ido Segev
In-Reply-To: <20201123190859.21298-1-shayagr@amazon.com>
After request id is checked in validate_rx_req_id() its value is still
used in the line
rx_ring->free_ids[next_to_clean] =
rx_ring->ena_bufs[i].req_id;
even if it was found to be out-of-bound for the array free_ids.
The patch moves the request id to an earlier stage in the napi routine and
makes sure its value isn't used if it's found out-of-bounds.
Fixes: 30623e1ed116 ("net: ena: avoid memory access violation by validating req_id properly")
Signed-off-by: Ido Segev <idose@amazon.com>
Signed-off-by: Shay Agroskin <shayagr@amazon.com>
---
drivers/net/ethernet/amazon/ena/ena_eth_com.c | 3 ++
drivers/net/ethernet/amazon/ena/ena_netdev.c | 43 +++++--------------
2 files changed, 14 insertions(+), 32 deletions(-)
diff --git a/drivers/net/ethernet/amazon/ena/ena_eth_com.c b/drivers/net/ethernet/amazon/ena/ena_eth_com.c
index ad30cacc1622..032ab9f20438 100644
--- a/drivers/net/ethernet/amazon/ena/ena_eth_com.c
+++ b/drivers/net/ethernet/amazon/ena/ena_eth_com.c
@@ -516,6 +516,7 @@ int ena_com_rx_pkt(struct ena_com_io_cq *io_cq,
{
struct ena_com_rx_buf_info *ena_buf = &ena_rx_ctx->ena_bufs[0];
struct ena_eth_io_rx_cdesc_base *cdesc = NULL;
+ u16 q_depth = io_cq->q_depth;
u16 cdesc_idx = 0;
u16 nb_hw_desc;
u16 i = 0;
@@ -543,6 +544,8 @@ int ena_com_rx_pkt(struct ena_com_io_cq *io_cq,
do {
ena_buf[i].len = cdesc->length;
ena_buf[i].req_id = cdesc->req_id;
+ if (unlikely(ena_buf[i].req_id >= q_depth))
+ return -EIO;
if (++i >= nb_hw_desc)
break;
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index e8131dadc22c..574c2b5ba21e 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -789,24 +789,6 @@ static void ena_free_all_io_tx_resources(struct ena_adapter *adapter)
adapter->num_io_queues);
}
-static int validate_rx_req_id(struct ena_ring *rx_ring, u16 req_id)
-{
- if (likely(req_id < rx_ring->ring_size))
- return 0;
-
- netif_err(rx_ring->adapter, rx_err, rx_ring->netdev,
- "Invalid rx req_id: %hu\n", req_id);
-
- u64_stats_update_begin(&rx_ring->syncp);
- rx_ring->rx_stats.bad_req_id++;
- u64_stats_update_end(&rx_ring->syncp);
-
- /* Trigger device reset */
- rx_ring->adapter->reset_reason = ENA_REGS_RESET_INV_RX_REQ_ID;
- set_bit(ENA_FLAG_TRIGGER_RESET, &rx_ring->adapter->flags);
- return -EFAULT;
-}
-
/* ena_setup_rx_resources - allocate I/O Rx resources (Descriptors)
* @adapter: network interface device structure
* @qid: queue index
@@ -1356,15 +1338,10 @@ static struct sk_buff *ena_rx_skb(struct ena_ring *rx_ring,
struct ena_rx_buffer *rx_info;
u16 len, req_id, buf = 0;
void *va;
- int rc;
len = ena_bufs[buf].len;
req_id = ena_bufs[buf].req_id;
- rc = validate_rx_req_id(rx_ring, req_id);
- if (unlikely(rc < 0))
- return NULL;
-
rx_info = &rx_ring->rx_buffer_info[req_id];
if (unlikely(!rx_info->page)) {
@@ -1440,10 +1417,6 @@ static struct sk_buff *ena_rx_skb(struct ena_ring *rx_ring,
len = ena_bufs[buf].len;
req_id = ena_bufs[buf].req_id;
- rc = validate_rx_req_id(rx_ring, req_id);
- if (unlikely(rc < 0))
- return NULL;
-
rx_info = &rx_ring->rx_buffer_info[req_id];
} while (1);
@@ -1697,12 +1670,18 @@ static int ena_clean_rx_irq(struct ena_ring *rx_ring, struct napi_struct *napi,
error:
adapter = netdev_priv(rx_ring->netdev);
- u64_stats_update_begin(&rx_ring->syncp);
- rx_ring->rx_stats.bad_desc_num++;
- u64_stats_update_end(&rx_ring->syncp);
+ if (rc == -ENOSPC) {
+ u64_stats_update_begin(&rx_ring->syncp);
+ rx_ring->rx_stats.bad_desc_num++;
+ u64_stats_update_end(&rx_ring->syncp);
+ adapter->reset_reason = ENA_REGS_RESET_TOO_MANY_RX_DESCS;
+ } else {
+ u64_stats_update_begin(&rx_ring->syncp);
+ rx_ring->rx_stats.bad_req_id++;
+ u64_stats_update_end(&rx_ring->syncp);
+ adapter->reset_reason = ENA_REGS_RESET_INV_RX_REQ_ID;
+ }
- /* Too many desc from the device. Trigger reset */
- adapter->reset_reason = ENA_REGS_RESET_TOO_MANY_RX_DESCS;
set_bit(ENA_FLAG_TRIGGER_RESET, &adapter->flags);
return 0;
--
2.17.1
^ permalink raw reply related
* [PATCH V3 net 2/3] net: ena: set initial DMA width to avoid intel iommu issue
From: Shay Agroskin @ 2020-11-23 19:08 UTC (permalink / raw)
To: kuba, netdev
Cc: Shay Agroskin, dwmw, zorik, matua, saeedb, msw, aliguori, nafea,
gtzalik, netanel, alisaidi, benh, akiyano, sameehj, ndagan,
Mike Cui
In-Reply-To: <20201123190859.21298-1-shayagr@amazon.com>
The ENA driver uses the readless mechanism, which uses DMA, to find
out what the DMA mask is supposed to be.
If DMA is used without setting the dma_mask first, it causes the
Intel IOMMU driver to think that ENA is a 32-bit device and therefore
disables IOMMU passthrough permanently.
This patch sets the dma_mask to be ENA_MAX_PHYS_ADDR_SIZE_BITS=48
before readless initialization in
ena_device_init()->ena_com_mmio_reg_read_request_init(),
which is large enough to workaround the intel_iommu issue.
DMA mask is set again to the correct value after it's received from the
device after readless is initialized.
The patch also changes the driver to use dma_set_mask_and_coherent()
function instead of the two pci_set_dma_mask() and
pci_set_consistent_dma_mask() ones. Both methods achieve the same
effect.
Fixes: 1738cd3ed342 ("net: ena: Add a driver for Amazon Elastic Network Adapters (ENA)")
Signed-off-by: Mike Cui <mikecui@amazon.com>
Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com>
Signed-off-by: Shay Agroskin <shayagr@amazon.com>
---
drivers/net/ethernet/amazon/ena/ena_netdev.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 574c2b5ba21e..ec0008ba7751 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -3367,16 +3367,9 @@ static int ena_device_init(struct ena_com_dev *ena_dev, struct pci_dev *pdev,
goto err_mmio_read_less;
}
- rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(dma_width));
+ rc = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(dma_width));
if (rc) {
- dev_err(dev, "pci_set_dma_mask failed 0x%x\n", rc);
- goto err_mmio_read_less;
- }
-
- rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(dma_width));
- if (rc) {
- dev_err(dev, "err_pci_set_consistent_dma_mask failed 0x%x\n",
- rc);
+ dev_err(dev, "dma_set_mask_and_coherent failed %d\n", rc);
goto err_mmio_read_less;
}
@@ -4146,6 +4139,12 @@ static int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
return rc;
}
+ rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(ENA_MAX_PHYS_ADDR_SIZE_BITS));
+ if (rc) {
+ dev_err(&pdev->dev, "dma_set_mask_and_coherent failed %d\n", rc);
+ goto err_disable_device;
+ }
+
pci_set_master(pdev);
ena_dev = vzalloc(sizeof(*ena_dev));
--
2.17.1
^ permalink raw reply related
* [PATCH V3 net 3/3] net: ena: fix packet's addresses for rx_offset feature
From: Shay Agroskin @ 2020-11-23 19:08 UTC (permalink / raw)
To: kuba, netdev
Cc: Shay Agroskin, dwmw, zorik, matua, saeedb, msw, aliguori, nafea,
gtzalik, netanel, alisaidi, benh, akiyano, sameehj, ndagan
In-Reply-To: <20201123190859.21298-1-shayagr@amazon.com>
This patch fixes two lines in which the rx_offset received by the device
wasn't taken into account:
- prefetch function:
In our driver the copied data would reside in
rx_info->page + rx_headroom + rx_offset
so the prefetch function is changed accordingly.
- setting page_offset to zero for descriptors > 1:
for every descriptor but the first, the rx_offset is zero. Hence
the page_offset value should be set to rx_headroom.
The previous implementation changed the value of rx_info after
the descriptor was added to the SKB (essentially providing wrong
page offset).
Fixes: 68f236df93a9 ("net: ena: add support for the rx offset feature")
Signed-off-by: Shay Agroskin <shayagr@amazon.com>
---
drivers/net/ethernet/amazon/ena/ena_netdev.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index ec0008ba7751..df1884d57d1a 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -908,10 +908,14 @@ static void ena_free_all_io_rx_resources(struct ena_adapter *adapter)
static int ena_alloc_rx_page(struct ena_ring *rx_ring,
struct ena_rx_buffer *rx_info, gfp_t gfp)
{
+ int headroom = rx_ring->rx_headroom;
struct ena_com_buf *ena_buf;
struct page *page;
dma_addr_t dma;
+ /* restore page offset value in case it has been changed by device */
+ rx_info->page_offset = headroom;
+
/* if previous allocated page is not used */
if (unlikely(rx_info->page))
return 0;
@@ -941,10 +945,9 @@ static int ena_alloc_rx_page(struct ena_ring *rx_ring,
"Allocate page %p, rx_info %p\n", page, rx_info);
rx_info->page = page;
- rx_info->page_offset = 0;
ena_buf = &rx_info->ena_buf;
- ena_buf->paddr = dma + rx_ring->rx_headroom;
- ena_buf->len = ENA_PAGE_SIZE - rx_ring->rx_headroom;
+ ena_buf->paddr = dma + headroom;
+ ena_buf->len = ENA_PAGE_SIZE - headroom;
return 0;
}
@@ -1356,7 +1359,8 @@ static struct sk_buff *ena_rx_skb(struct ena_ring *rx_ring,
/* save virt address of first buffer */
va = page_address(rx_info->page) + rx_info->page_offset;
- prefetch(va + NET_IP_ALIGN);
+
+ prefetch(va);
if (len <= rx_ring->rx_copybreak) {
skb = ena_alloc_skb(rx_ring, false);
@@ -1397,8 +1401,6 @@ static struct sk_buff *ena_rx_skb(struct ena_ring *rx_ring,
skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, rx_info->page,
rx_info->page_offset, len, ENA_PAGE_SIZE);
- /* The offset is non zero only for the first buffer */
- rx_info->page_offset = 0;
netif_dbg(rx_ring->adapter, rx_status, rx_ring->netdev,
"RX skb updated. len %d. data_len %d\n",
@@ -1517,8 +1519,7 @@ static int ena_xdp_handle_buff(struct ena_ring *rx_ring, struct xdp_buff *xdp)
int ret;
rx_info = &rx_ring->rx_buffer_info[rx_ring->ena_bufs[0].req_id];
- xdp->data = page_address(rx_info->page) +
- rx_info->page_offset + rx_ring->rx_headroom;
+ xdp->data = page_address(rx_info->page) + rx_info->page_offset;
xdp_set_data_meta_invalid(xdp);
xdp->data_hard_start = page_address(rx_info->page);
xdp->data_end = xdp->data + rx_ring->ena_bufs[0].len;
@@ -1585,8 +1586,9 @@ static int ena_clean_rx_irq(struct ena_ring *rx_ring, struct napi_struct *napi,
if (unlikely(ena_rx_ctx.descs == 0))
break;
+ /* First descriptor might have an offset set by the device */
rx_info = &rx_ring->rx_buffer_info[rx_ring->ena_bufs[0].req_id];
- rx_info->page_offset = ena_rx_ctx.pkt_offset;
+ rx_info->page_offset += ena_rx_ctx.pkt_offset;
netif_dbg(rx_ring->adapter, rx_status, rx_ring->netdev,
"rx_poll: q %d got packet from ena. descs #: %d l3 proto %d l4 proto %d hash: %x\n",
--
2.17.1
^ 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