* [PATCH bpf-next v6 5/5] devmap: Allow map lookups from eBPF
From: Toke Høiland-Jørgensen @ 2019-06-28 9:12 UTC (permalink / raw)
To: netdev
Cc: Jesper Dangaard Brouer, Daniel Borkmann, Alexei Starovoitov,
David Miller, Jonathan Lemon
In-Reply-To: <156171315462.9468.3367572649463706996.stgit@alrua-x1>
From: Toke Høiland-Jørgensen <toke@redhat.com>
We don't currently allow lookups into a devmap from eBPF, because the map
lookup returns a pointer directly to the dev->ifindex, which shouldn't be
modifiable from eBPF.
However, being able to do lookups in devmaps is useful to know (e.g.)
whether forwarding to a specific interface is enabled. Currently, programs
work around this by keeping a shadow map of another type which indicates
whether a map index is valid.
Since we now have a flag to make maps read-only from the eBPF side, we can
simply lift the lookup restriction if we make sure this flag is always set.
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>
Acked-by: Andrii Nakryiko <andriin@fb.com>
---
kernel/bpf/devmap.c | 5 +++++
kernel/bpf/verifier.c | 7 ++-----
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index a4dddc867cbf..d83cf8ccc872 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -89,6 +89,11 @@ static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
attr->value_size != 4 || attr->map_flags & ~DEV_CREATE_FLAG_MASK)
return ERR_PTR(-EINVAL);
+ /* Lookup returns a pointer straight to dev->ifindex, so make sure the
+ * verifier prevents writes from the BPF side
+ */
+ attr->map_flags |= BPF_F_RDONLY_PROG;
+
dtab = kzalloc(sizeof(*dtab), GFP_USER);
if (!dtab)
return ERR_PTR(-ENOMEM);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 0e079b2298f8..51c327ee7d3f 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3407,12 +3407,9 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
if (func_id != BPF_FUNC_get_local_storage)
goto error;
break;
- /* devmap returns a pointer to a live net_device ifindex that we cannot
- * allow to be modified from bpf side. So do not allow lookup elements
- * for now.
- */
case BPF_MAP_TYPE_DEVMAP:
- if (func_id != BPF_FUNC_redirect_map)
+ if (func_id != BPF_FUNC_redirect_map &&
+ func_id != BPF_FUNC_map_lookup_elem)
goto error;
break;
/* Restrict bpf side of cpumap and xskmap, open when use-cases
^ permalink raw reply related
* [PATCH bpf-next v6 3/5] devmap: Rename ifindex member in bpf_redirect_info
From: Toke Høiland-Jørgensen @ 2019-06-28 9:12 UTC (permalink / raw)
To: netdev
Cc: Jesper Dangaard Brouer, Daniel Borkmann, Alexei Starovoitov,
David Miller, Jonathan Lemon
In-Reply-To: <156171315462.9468.3367572649463706996.stgit@alrua-x1>
From: Toke Høiland-Jørgensen <toke@redhat.com>
The bpf_redirect_info struct has an 'ifindex' member which was named back
when the redirects could only target egress interfaces. Now that we can
also redirect to sockets and CPUs, this is a bit misleading, so rename the
member to tgt_index.
Reorder the struct members so we can have 'tgt_index' and 'tgt_value' next
to each other in a subsequent patch.
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
include/linux/filter.h | 2 +-
net/core/filter.c | 26 +++++++++++++-------------
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 43b45d6db36d..af8601340dae 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -578,8 +578,8 @@ struct bpf_skb_data_end {
};
struct bpf_redirect_info {
- u32 ifindex;
u32 flags;
+ u32 tgt_index;
struct bpf_map *map;
struct bpf_map *map_to_flush;
u32 kern_flags;
diff --git a/net/core/filter.c b/net/core/filter.c
index 183bf4d8e301..1d7f5f7f32cf 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2158,8 +2158,8 @@ BPF_CALL_2(bpf_redirect, u32, ifindex, u64, flags)
if (unlikely(flags & ~(BPF_F_INGRESS)))
return TC_ACT_SHOT;
- ri->ifindex = ifindex;
ri->flags = flags;
+ ri->tgt_index = ifindex;
return TC_ACT_REDIRECT;
}
@@ -2169,8 +2169,8 @@ int skb_do_redirect(struct sk_buff *skb)
struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
struct net_device *dev;
- dev = dev_get_by_index_rcu(dev_net(skb->dev), ri->ifindex);
- ri->ifindex = 0;
+ dev = dev_get_by_index_rcu(dev_net(skb->dev), ri->tgt_index);
+ ri->tgt_index = 0;
if (unlikely(!dev)) {
kfree_skb(skb);
return -EINVAL;
@@ -3488,11 +3488,11 @@ xdp_do_redirect_slow(struct net_device *dev, struct xdp_buff *xdp,
struct bpf_prog *xdp_prog, struct bpf_redirect_info *ri)
{
struct net_device *fwd;
- u32 index = ri->ifindex;
+ u32 index = ri->tgt_index;
int err;
fwd = dev_get_by_index_rcu(dev_net(dev), index);
- ri->ifindex = 0;
+ ri->tgt_index = 0;
if (unlikely(!fwd)) {
err = -EINVAL;
goto err;
@@ -3604,11 +3604,11 @@ static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
struct bpf_prog *xdp_prog, struct bpf_map *map,
struct bpf_redirect_info *ri)
{
- u32 index = ri->ifindex;
+ u32 index = ri->tgt_index;
void *fwd = NULL;
int err;
- ri->ifindex = 0;
+ ri->tgt_index = 0;
WRITE_ONCE(ri->map, NULL);
fwd = __xdp_map_lookup_elem(map, index);
@@ -3651,11 +3651,11 @@ static int xdp_do_generic_redirect_map(struct net_device *dev,
struct bpf_map *map)
{
struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
- u32 index = ri->ifindex;
+ u32 index = ri->tgt_index;
void *fwd = NULL;
int err = 0;
- ri->ifindex = 0;
+ ri->tgt_index = 0;
WRITE_ONCE(ri->map, NULL);
fwd = __xdp_map_lookup_elem(map, index);
@@ -3695,14 +3695,14 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
{
struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
struct bpf_map *map = READ_ONCE(ri->map);
- u32 index = ri->ifindex;
+ u32 index = ri->tgt_index;
struct net_device *fwd;
int err = 0;
if (map)
return xdp_do_generic_redirect_map(dev, skb, xdp, xdp_prog,
map);
- ri->ifindex = 0;
+ ri->tgt_index = 0;
fwd = dev_get_by_index_rcu(dev_net(dev), index);
if (unlikely(!fwd)) {
err = -EINVAL;
@@ -3730,8 +3730,8 @@ BPF_CALL_2(bpf_xdp_redirect, u32, ifindex, u64, flags)
if (unlikely(flags))
return XDP_ABORTED;
- ri->ifindex = ifindex;
ri->flags = flags;
+ ri->tgt_index = ifindex;
WRITE_ONCE(ri->map, NULL);
return XDP_REDIRECT;
@@ -3753,8 +3753,8 @@ BPF_CALL_3(bpf_xdp_redirect_map, struct bpf_map *, map, u32, ifindex,
if (unlikely(flags))
return XDP_ABORTED;
- ri->ifindex = ifindex;
ri->flags = flags;
+ ri->tgt_index = ifindex;
WRITE_ONCE(ri->map, map);
return XDP_REDIRECT;
^ permalink raw reply related
* [PATCH bpf-next v6 1/5] xskmap: Move non-standard list manipulation to helper
From: Toke Høiland-Jørgensen @ 2019-06-28 9:12 UTC (permalink / raw)
To: netdev
Cc: Jesper Dangaard Brouer, Daniel Borkmann, Alexei Starovoitov,
David Miller, Jonathan Lemon
In-Reply-To: <156171315462.9468.3367572649463706996.stgit@alrua-x1>
From: Toke Høiland-Jørgensen <toke@redhat.com>
Add a helper in list.h for the non-standard way of clearing a list that is
used in xskmap. This makes it easier to reuse it in the other map types,
and also makes sure this usage is not forgotten in any list refactorings in
the future.
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
include/linux/list.h | 14 ++++++++++++++
kernel/bpf/xskmap.c | 3 +--
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/include/linux/list.h b/include/linux/list.h
index e951228db4b2..85c92555e31f 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -106,6 +106,20 @@ static inline void __list_del(struct list_head * prev, struct list_head * next)
WRITE_ONCE(prev->next, next);
}
+/*
+ * Delete a list entry and clear the 'prev' pointer.
+ *
+ * This is a special-purpose list clearing method used in the networking code
+ * for lists allocated as per-cpu, where we don't want to incur the extra
+ * WRITE_ONCE() overhead of a regular list_del_init(). The code that uses this
+ * needs to check the node 'prev' pointer instead of calling list_empty().
+ */
+static inline void __list_del_clearprev(struct list_head *entry)
+{
+ __list_del(entry->prev, entry->next);
+ entry->prev = NULL;
+}
+
/**
* list_del - deletes entry from list.
* @entry: the element to delete from the list.
diff --git a/kernel/bpf/xskmap.c b/kernel/bpf/xskmap.c
index ef7338cebd18..9bb96ace9fa1 100644
--- a/kernel/bpf/xskmap.c
+++ b/kernel/bpf/xskmap.c
@@ -145,8 +145,7 @@ void __xsk_map_flush(struct bpf_map *map)
list_for_each_entry_safe(xs, tmp, flush_list, flush_node) {
xsk_flush(xs);
- __list_del(xs->flush_node.prev, xs->flush_node.next);
- xs->flush_node.prev = NULL;
+ __list_del_clearprev(&xs->flush_node);
}
}
^ permalink raw reply related
* [PATCH 1/4 nf-next v2] netfilter:nf_flow_table: Refactor flow_offload_tuple to destination
From: wenxu @ 2019-06-28 9:48 UTC (permalink / raw)
To: pablo, fw; +Cc: netfilter-devel, netdev
From: wenxu <wenxu@ucloud.cn>
Add struct flow_offload_dst to support more offload method to replace
dst_cache which only work for route offload.
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
include/net/netfilter/nf_flow_table.h | 12 ++++++++++--
net/netfilter/nf_flow_table_core.c | 22 +++++++++++-----------
net/netfilter/nf_flow_table_ip.c | 4 ++--
net/netfilter/nft_flow_offload.c | 12 ++++++------
4 files changed, 29 insertions(+), 21 deletions(-)
diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h
index d8c1879..d40d409 100644
--- a/include/net/netfilter/nf_flow_table.h
+++ b/include/net/netfilter/nf_flow_table.h
@@ -33,6 +33,10 @@ enum flow_offload_tuple_dir {
FLOW_OFFLOAD_DIR_MAX = IP_CT_DIR_MAX
};
+struct flow_offload_dst {
+ struct dst_entry *dst_cache;
+};
+
struct flow_offload_tuple {
union {
struct in_addr src_v4;
@@ -55,7 +59,7 @@ struct flow_offload_tuple {
u16 mtu;
- struct dst_entry *dst_cache;
+ struct flow_offload_dst dst;
};
struct flow_offload_tuple_rhash {
@@ -85,8 +89,12 @@ struct nf_flow_route {
} tuple[FLOW_OFFLOAD_DIR_MAX];
};
+struct nf_flow_dst {
+ struct nf_flow_route route;
+};
+
struct flow_offload *flow_offload_alloc(struct nf_conn *ct,
- struct nf_flow_route *route);
+ struct nf_flow_dst *flow_dst);
void flow_offload_free(struct flow_offload *flow);
int flow_offload_add(struct nf_flowtable *flow_table, struct flow_offload *flow);
diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
index e3d7972..7e0b5bd 100644
--- a/net/netfilter/nf_flow_table_core.c
+++ b/net/netfilter/nf_flow_table_core.c
@@ -24,13 +24,13 @@ struct flow_offload_entry {
static void
flow_offload_fill_dir(struct flow_offload *flow, struct nf_conn *ct,
- struct nf_flow_route *route,
+ struct nf_flow_dst *flow_dst,
enum flow_offload_tuple_dir dir)
{
struct flow_offload_tuple *ft = &flow->tuplehash[dir].tuple;
struct nf_conntrack_tuple *ctt = &ct->tuplehash[dir].tuple;
- struct dst_entry *other_dst = route->tuple[!dir].dst;
- struct dst_entry *dst = route->tuple[dir].dst;
+ struct dst_entry *other_dst = flow_dst->route.tuple[!dir].dst;
+ struct dst_entry *dst = flow_dst->route.tuple[dir].dst;
ft->dir = dir;
@@ -57,7 +57,7 @@ struct flow_offload_entry {
}
struct flow_offload *
-flow_offload_alloc(struct nf_conn *ct, struct nf_flow_route *route)
+flow_offload_alloc(struct nf_conn *ct, struct nf_flow_dst *flow_dst)
{
struct flow_offload_entry *entry;
struct flow_offload *flow;
@@ -72,16 +72,16 @@ struct flow_offload *
flow = &entry->flow;
- if (!dst_hold_safe(route->tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst))
+ if (!dst_hold_safe(flow_dst->route.tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst))
goto err_dst_cache_original;
- if (!dst_hold_safe(route->tuple[FLOW_OFFLOAD_DIR_REPLY].dst))
+ if (!dst_hold_safe(flow_dst->route.tuple[FLOW_OFFLOAD_DIR_REPLY].dst))
goto err_dst_cache_reply;
entry->ct = ct;
- flow_offload_fill_dir(flow, ct, route, FLOW_OFFLOAD_DIR_ORIGINAL);
- flow_offload_fill_dir(flow, ct, route, FLOW_OFFLOAD_DIR_REPLY);
+ flow_offload_fill_dir(flow, ct, flow_dst, FLOW_OFFLOAD_DIR_ORIGINAL);
+ flow_offload_fill_dir(flow, ct, flow_dst, FLOW_OFFLOAD_DIR_REPLY);
if (ct->status & IPS_SRC_NAT)
flow->flags |= FLOW_OFFLOAD_SNAT;
@@ -91,7 +91,7 @@ struct flow_offload *
return flow;
err_dst_cache_reply:
- dst_release(route->tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst);
+ dst_release(flow_dst->route.tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst);
err_dst_cache_original:
kfree(entry);
err_ct_refcnt:
@@ -139,8 +139,8 @@ void flow_offload_free(struct flow_offload *flow)
{
struct flow_offload_entry *e;
- dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.dst_cache);
- dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst_cache);
+ dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.dst.dst_cache);
+ dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst.dst_cache);
e = container_of(flow, struct flow_offload_entry, flow);
if (flow->flags & FLOW_OFFLOAD_DYING)
nf_ct_delete(e->ct, 0, 0);
diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
index 2413174..0016bb8 100644
--- a/net/netfilter/nf_flow_table_ip.c
+++ b/net/netfilter/nf_flow_table_ip.c
@@ -241,7 +241,7 @@ static bool nf_flow_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
dir = tuplehash->tuple.dir;
flow = container_of(tuplehash, struct flow_offload, tuplehash[dir]);
- rt = (struct rtable *)flow->tuplehash[dir].tuple.dst_cache;
+ rt = (struct rtable *)flow->tuplehash[dir].tuple.dst.dst_cache;
outdev = rt->dst.dev;
if (unlikely(nf_flow_exceeds_mtu(skb, flow->tuplehash[dir].tuple.mtu)))
@@ -457,7 +457,7 @@ static int nf_flow_tuple_ipv6(struct sk_buff *skb, const struct net_device *dev,
dir = tuplehash->tuple.dir;
flow = container_of(tuplehash, struct flow_offload, tuplehash[dir]);
- rt = (struct rt6_info *)flow->tuplehash[dir].tuple.dst_cache;
+ rt = (struct rt6_info *)flow->tuplehash[dir].tuple.dst.dst_cache;
outdev = rt->dst.dev;
if (unlikely(nf_flow_exceeds_mtu(skb, flow->tuplehash[dir].tuple.mtu)))
diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
index aa5f571..261c565 100644
--- a/net/netfilter/nft_flow_offload.c
+++ b/net/netfilter/nft_flow_offload.c
@@ -70,10 +70,10 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
struct nft_regs *regs,
const struct nft_pktinfo *pkt)
{
- struct nft_flow_offload *priv = nft_expr_priv(expr);
struct nf_flowtable *flowtable = &priv->flowtable->data;
+ struct nft_flow_offload *priv = nft_expr_priv(expr);
enum ip_conntrack_info ctinfo;
- struct nf_flow_route route;
+ struct nf_flow_dst flow_dst;
struct flow_offload *flow;
enum ip_conntrack_dir dir;
bool is_tcp = false;
@@ -108,10 +108,10 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
goto out;
dir = CTINFO2DIR(ctinfo);
- if (nft_flow_route(pkt, ct, &route, dir) < 0)
+ if (nft_flow_route(pkt, ct, &flow_dst.route, dir) < 0)
goto err_flow_route;
- flow = flow_offload_alloc(ct, &route);
+ flow = flow_offload_alloc(ct, &flow_dst);
if (!flow)
goto err_flow_alloc;
@@ -124,13 +124,13 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
if (ret < 0)
goto err_flow_add;
- dst_release(route.tuple[!dir].dst);
+ dst_release(flow_dst.route.tuple[!dir].dst);
return;
err_flow_add:
flow_offload_free(flow);
err_flow_alloc:
- dst_release(route.tuple[!dir].dst);
+ dst_release(flow_dst.route.tuple[!dir].dst);
err_flow_route:
clear_bit(IPS_OFFLOAD_BIT, &ct->status);
out:
--
1.8.3.1
^ permalink raw reply related
* [PATCH 2/4 nf-next v2] netfilter:nf_flow_table: Separate inet operation to single function
From: wenxu @ 2019-06-28 9:48 UTC (permalink / raw)
To: pablo, fw; +Cc: netfilter-devel, netdev
In-Reply-To: <1561715326-3945-1-git-send-email-wenxu@ucloud.cn>
From: wenxu <wenxu@ucloud.cn>
This patch separate the inet family operation to single function.
Prepare for supporting the bridge family.
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
net/netfilter/nf_flow_table_core.c | 52 ++++++++++++++++++++++++++++----------
net/netfilter/nf_flow_table_ip.c | 34 +++++++++++++++----------
net/netfilter/nft_flow_offload.c | 2 +-
3 files changed, 60 insertions(+), 28 deletions(-)
diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
index 7e0b5bd..2bec409 100644
--- a/net/netfilter/nf_flow_table_core.c
+++ b/net/netfilter/nf_flow_table_core.c
@@ -22,6 +22,20 @@ struct flow_offload_entry {
static DEFINE_MUTEX(flowtable_lock);
static LIST_HEAD(flowtables);
+static struct dst_entry *
+flow_offload_fill_inet_dst(struct flow_offload_tuple *ft,
+ struct nf_flow_route *route,
+ enum flow_offload_tuple_dir dir)
+{
+ struct dst_entry *other_dst = route->tuple[!dir].dst;
+ struct dst_entry *dst = route->tuple[dir].dst;
+
+ ft->iifidx = other_dst->dev->ifindex;
+ ft->dst.dst_cache = dst;
+
+ return dst;
+}
+
static void
flow_offload_fill_dir(struct flow_offload *flow, struct nf_conn *ct,
struct nf_flow_dst *flow_dst,
@@ -29,9 +43,9 @@ struct flow_offload_entry {
{
struct flow_offload_tuple *ft = &flow->tuplehash[dir].tuple;
struct nf_conntrack_tuple *ctt = &ct->tuplehash[dir].tuple;
- struct dst_entry *other_dst = flow_dst->route.tuple[!dir].dst;
- struct dst_entry *dst = flow_dst->route.tuple[dir].dst;
+ struct dst_entry *dst;
+ dst = flow_offload_fill_inet_dst(ft, &flow_dst->route, dir);
ft->dir = dir;
switch (ctt->src.l3num) {
@@ -51,9 +65,19 @@ struct flow_offload_entry {
ft->l4proto = ctt->dst.protonum;
ft->src_port = ctt->src.u.tcp.port;
ft->dst_port = ctt->dst.u.tcp.port;
+}
- ft->iifidx = other_dst->dev->ifindex;
- ft->dst_cache = dst;
+static int flow_offload_dst_hold(struct nf_flow_dst *flow_dst)
+{
+ if (!dst_hold_safe(flow_dst->route.tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst))
+ return -1;
+
+ if (!dst_hold_safe(flow_dst->route.tuple[FLOW_OFFLOAD_DIR_REPLY].dst)) {
+ dst_release(flow_dst->route.tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst);
+ return -1;
+ }
+
+ return 0;
}
struct flow_offload *
@@ -72,11 +96,8 @@ struct flow_offload *
flow = &entry->flow;
- if (!dst_hold_safe(flow_dst->route.tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst))
- goto err_dst_cache_original;
-
- if (!dst_hold_safe(flow_dst->route.tuple[FLOW_OFFLOAD_DIR_REPLY].dst))
- goto err_dst_cache_reply;
+ if (flow_offload_dst_hold(flow_dst))
+ goto err_dst_cache;
entry->ct = ct;
@@ -90,9 +111,7 @@ struct flow_offload *
return flow;
-err_dst_cache_reply:
- dst_release(flow_dst->route.tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst);
-err_dst_cache_original:
+err_dst_cache:
kfree(entry);
err_ct_refcnt:
nf_ct_put(ct);
@@ -135,12 +154,17 @@ static void flow_offload_fixup_ct_state(struct nf_conn *ct)
ct->timeout = nfct_time_stamp + timeout;
}
+static void flow_offload_dst_release(struct flow_offload *flow)
+{
+ dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.dst.dst_cache);
+ dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst.dst_cache);
+}
+
void flow_offload_free(struct flow_offload *flow)
{
struct flow_offload_entry *e;
- dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.dst.dst_cache);
- dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst.dst_cache);
+ flow_offload_dst_release(flow);
e = container_of(flow, struct flow_offload_entry, flow);
if (flow->flags & FLOW_OFFLOAD_DYING)
nf_ct_delete(e->ct, 0, 0);
diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
index 0016bb8..24263e2 100644
--- a/net/netfilter/nf_flow_table_ip.c
+++ b/net/netfilter/nf_flow_table_ip.c
@@ -214,6 +214,25 @@ static bool nf_flow_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
return true;
}
+static void nf_flow_inet_xmit(struct flow_offload *flow, struct sk_buff *skb,
+ enum flow_offload_tuple_dir dir)
+{
+ struct net_device *outdev;
+ struct rtable *rt;
+ struct iphdr *iph;
+ __be32 nexthop;
+
+ rt = (struct rtable *)flow->tuplehash[dir].tuple.dst.dst_cache;
+ outdev = rt->dst.dev;
+ iph = ip_hdr(skb);
+ ip_decrease_ttl(iph);
+
+ skb->dev = outdev;
+ nexthop = rt_nexthop(rt, flow->tuplehash[!dir].tuple.src_v4.s_addr);
+ skb_dst_set_noref(skb, &rt->dst);
+ neigh_xmit(NEIGH_ARP_TABLE, outdev, &nexthop, skb);
+}
+
unsigned int
nf_flow_offload_ip_hook(void *priv, struct sk_buff *skb,
const struct nf_hook_state *state)
@@ -223,11 +242,7 @@ static bool nf_flow_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
struct flow_offload_tuple tuple = {};
enum flow_offload_tuple_dir dir;
struct flow_offload *flow;
- struct net_device *outdev;
- struct rtable *rt;
unsigned int thoff;
- struct iphdr *iph;
- __be32 nexthop;
if (skb->protocol != htons(ETH_P_IP))
return NF_ACCEPT;
@@ -241,13 +256,11 @@ static bool nf_flow_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
dir = tuplehash->tuple.dir;
flow = container_of(tuplehash, struct flow_offload, tuplehash[dir]);
- rt = (struct rtable *)flow->tuplehash[dir].tuple.dst.dst_cache;
- outdev = rt->dst.dev;
if (unlikely(nf_flow_exceeds_mtu(skb, flow->tuplehash[dir].tuple.mtu)))
return NF_ACCEPT;
- if (skb_try_make_writable(skb, sizeof(*iph)))
+ if (skb_try_make_writable(skb, sizeof(struct iphdr)))
return NF_DROP;
thoff = ip_hdr(skb)->ihl * 4;
@@ -258,13 +271,8 @@ static bool nf_flow_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
return NF_DROP;
flow->timeout = (u32)jiffies + NF_FLOW_TIMEOUT;
- iph = ip_hdr(skb);
- ip_decrease_ttl(iph);
- skb->dev = outdev;
- nexthop = rt_nexthop(rt, flow->tuplehash[!dir].tuple.src_v4.s_addr);
- skb_dst_set_noref(skb, &rt->dst);
- neigh_xmit(NEIGH_ARP_TABLE, outdev, &nexthop, skb);
+ nf_flow_inet_xmit(flow, skb, dir);
return NF_STOLEN;
}
diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
index 261c565..4af94ce 100644
--- a/net/netfilter/nft_flow_offload.c
+++ b/net/netfilter/nft_flow_offload.c
@@ -70,8 +70,8 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
struct nft_regs *regs,
const struct nft_pktinfo *pkt)
{
- struct nf_flowtable *flowtable = &priv->flowtable->data;
struct nft_flow_offload *priv = nft_expr_priv(expr);
+ struct nf_flowtable *flowtable = &priv->flowtable->data;
enum ip_conntrack_info ctinfo;
struct nf_flow_dst flow_dst;
struct flow_offload *flow;
--
1.8.3.1
^ permalink raw reply related
* [PATCH 3/4 nf-next v2] netfilter:nf_flow_table: Support bridge family flow offload
From: wenxu @ 2019-06-28 9:48 UTC (permalink / raw)
To: pablo, fw; +Cc: netfilter-devel, netdev
In-Reply-To: <1561715326-3945-1-git-send-email-wenxu@ucloud.cn>
From: wenxu <wenxu@ucloud.cn>
With nf_conntrack_bridge function. The bridge family can do
conntrack it self. The flow offload function based on the
conntrack. So the flow in the bridge wih conntrack can be
offloaded.
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
include/net/netfilter/nf_flow_table.h | 31 +++++++-
net/bridge/br_vlan.c | 1 +
net/netfilter/nf_flow_table_core.c | 58 ++++++++++++---
net/netfilter/nf_flow_table_ip.c | 43 +++++++++++-
net/netfilter/nft_flow_offload.c | 129 ++++++++++++++++++++++++++++++++--
5 files changed, 243 insertions(+), 19 deletions(-)
diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h
index d40d409..dcf197a 100644
--- a/include/net/netfilter/nf_flow_table.h
+++ b/include/net/netfilter/nf_flow_table.h
@@ -33,8 +33,23 @@ enum flow_offload_tuple_dir {
FLOW_OFFLOAD_DIR_MAX = IP_CT_DIR_MAX
};
+enum flow_offload_tuple_type {
+ FLOW_OFFLOAD_TYPE_INET,
+ FLOW_OFFLOAD_TYPE_BRIDGE,
+};
+
+struct dst_br_port {
+ struct net_device *dev;
+ u16 dst_vlan_tag;
+ u16 vlan_proto;
+};
+
struct flow_offload_dst {
- struct dst_entry *dst_cache;
+ enum flow_offload_tuple_type type;
+ union {
+ struct dst_entry *dst_cache;
+ struct dst_br_port dst_port;
+ };
};
struct flow_offload_tuple {
@@ -52,6 +67,7 @@ struct flow_offload_tuple {
};
int iifidx;
+ u16 vlan_tag;
u8 l3proto;
u8 l4proto;
@@ -89,8 +105,19 @@ struct nf_flow_route {
} tuple[FLOW_OFFLOAD_DIR_MAX];
};
+struct nf_flow_forward {
+ struct {
+ struct dst_br_port dst_port;
+ u16 vlan_tag;
+ } tuple[FLOW_OFFLOAD_DIR_MAX];
+};
+
struct nf_flow_dst {
- struct nf_flow_route route;
+ enum flow_offload_tuple_type type;
+ union {
+ struct nf_flow_route route;
+ struct nf_flow_forward forward;
+ };
};
struct flow_offload *flow_offload_alloc(struct nf_conn *ct,
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index f47f526..bcb7ce4 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -729,6 +729,7 @@ struct net_bridge_vlan *br_vlan_find(struct net_bridge_vlan_group *vg, u16 vid)
return br_vlan_lookup(&vg->vlan_hash, vid);
}
+EXPORT_SYMBOL_GPL(br_vlan_find);
/* Must be protected by RTNL. */
static void recalculate_group_addr(struct net_bridge *br)
diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
index 2bec409..08c1ca4 100644
--- a/net/netfilter/nf_flow_table_core.c
+++ b/net/netfilter/nf_flow_table_core.c
@@ -36,6 +36,21 @@ struct flow_offload_entry {
return dst;
}
+static struct dst_br_port *
+flow_offload_fill_bridge_dst(struct flow_offload_tuple *ft,
+ struct nf_flow_forward *forward,
+ enum flow_offload_tuple_dir dir)
+{
+ struct dst_br_port other_dst_port = forward->tuple[!dir].dst_port;
+ struct dst_br_port dst_port = forward->tuple[dir].dst_port;
+
+ ft->iifidx = other_dst_port.dev->ifindex;
+ ft->dst.dst_port = dst_port;
+ ft->vlan_tag = forward->tuple[dir].vlan_tag;
+
+ return &ft->dst.dst_port;
+}
+
static void
flow_offload_fill_dir(struct flow_offload *flow, struct nf_conn *ct,
struct nf_flow_dst *flow_dst,
@@ -43,16 +58,29 @@ struct flow_offload_entry {
{
struct flow_offload_tuple *ft = &flow->tuplehash[dir].tuple;
struct nf_conntrack_tuple *ctt = &ct->tuplehash[dir].tuple;
+ struct dst_br_port *dst_port;
struct dst_entry *dst;
- dst = flow_offload_fill_inet_dst(ft, &flow_dst->route, dir);
+ switch (flow_dst->type) {
+ case FLOW_OFFLOAD_TYPE_INET:
+ dst = flow_offload_fill_inet_dst(ft, &flow_dst->route, dir);
+ break;
+ case FLOW_OFFLOAD_TYPE_BRIDGE:
+ dst_port = flow_offload_fill_bridge_dst(ft, &flow_dst->forward, dir);
+ break;
+ }
+
+ ft->dst.type = flow_dst->type;
ft->dir = dir;
switch (ctt->src.l3num) {
case NFPROTO_IPV4:
ft->src_v4 = ctt->src.u3.in;
ft->dst_v4 = ctt->dst.u3.in;
- ft->mtu = ip_dst_mtu_maybe_forward(dst, true);
+ if (flow_dst->type == FLOW_OFFLOAD_TYPE_INET)
+ ft->mtu = ip_dst_mtu_maybe_forward(dst, true);
+ else
+ ft->mtu = dst_port->dev->mtu;
break;
case NFPROTO_IPV6:
ft->src_v6 = ctt->src.u3.in6;
@@ -67,13 +95,13 @@ struct flow_offload_entry {
ft->dst_port = ctt->dst.u.tcp.port;
}
-static int flow_offload_dst_hold(struct nf_flow_dst *flow_dst)
+static int flow_offload_dst_hold(struct nf_flow_route *route)
{
- if (!dst_hold_safe(flow_dst->route.tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst))
+ if (!dst_hold_safe(route->tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst))
return -1;
- if (!dst_hold_safe(flow_dst->route.tuple[FLOW_OFFLOAD_DIR_REPLY].dst)) {
- dst_release(flow_dst->route.tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst);
+ if (!dst_hold_safe(route->tuple[FLOW_OFFLOAD_DIR_REPLY].dst)) {
+ dst_release(route->tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst);
return -1;
}
@@ -96,7 +124,8 @@ struct flow_offload *
flow = &entry->flow;
- if (flow_offload_dst_hold(flow_dst))
+ if (flow_dst->type == FLOW_OFFLOAD_TYPE_INET &&
+ flow_offload_dst_hold(&flow_dst->route))
goto err_dst_cache;
entry->ct = ct;
@@ -156,8 +185,19 @@ static void flow_offload_fixup_ct_state(struct nf_conn *ct)
static void flow_offload_dst_release(struct flow_offload *flow)
{
- dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.dst.dst_cache);
- dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst.dst_cache);
+ enum flow_offload_tuple_type type = flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.dst.type;
+
+ switch (type) {
+ case FLOW_OFFLOAD_TYPE_INET:
+ dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.dst.dst_cache);
+ dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst.dst_cache);
+ break;
+
+ case FLOW_OFFLOAD_TYPE_BRIDGE:
+ dev_put(flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.dst.dst_port.dev);
+ dev_put(flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst.dst_port.dev);
+ break;
+ }
}
void flow_offload_free(struct flow_offload *flow)
diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
index 24263e2..490b4be 100644
--- a/net/netfilter/nf_flow_table_ip.c
+++ b/net/netfilter/nf_flow_table_ip.c
@@ -233,12 +233,40 @@ static void nf_flow_inet_xmit(struct flow_offload *flow, struct sk_buff *skb,
neigh_xmit(NEIGH_ARP_TABLE, outdev, &nexthop, skb);
}
+static void nf_flow_bridge_xmit(struct flow_offload *flow, struct sk_buff *skb,
+ enum flow_offload_tuple_dir dir)
+{
+ struct net_device *outdev;
+ u16 vlan_tag, vlan_proto;
+
+ vlan_tag = flow->tuplehash[dir].tuple.dst.dst_port.dst_vlan_tag;
+ vlan_proto = flow->tuplehash[dir].tuple.dst.dst_port.vlan_proto;
+ outdev = flow->tuplehash[dir].tuple.dst.dst_port.dev;
+ skb->dev = outdev;
+
+ if (vlan_tag)
+ __vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tag);
+ else
+ __vlan_hwaccel_clear_tag(skb);
+
+ skb_push(skb, ETH_HLEN);
+ if (!is_skb_forwardable(skb->dev, skb))
+ goto drop;
+
+ dev_queue_xmit(skb);
+ return;
+
+drop:
+ kfree_skb(skb);
+}
+
unsigned int
nf_flow_offload_ip_hook(void *priv, struct sk_buff *skb,
const struct nf_hook_state *state)
{
struct flow_offload_tuple_rhash *tuplehash;
struct nf_flowtable *flow_table = priv;
+ int family = flow_table->type->family;
struct flow_offload_tuple tuple = {};
enum flow_offload_tuple_dir dir;
struct flow_offload *flow;
@@ -247,9 +275,15 @@ static void nf_flow_inet_xmit(struct flow_offload *flow, struct sk_buff *skb,
if (skb->protocol != htons(ETH_P_IP))
return NF_ACCEPT;
+ if (family != NFPROTO_BRIDGE && family != NFPROTO_IPV4)
+ return NF_ACCEPT;
+
if (nf_flow_tuple_ip(skb, state->in, &tuple) < 0)
return NF_ACCEPT;
+ if (family == NFPROTO_BRIDGE && skb_vlan_tag_present(skb))
+ tuple.vlan_tag = skb_vlan_tag_get_id(skb);
+
tuplehash = flow_offload_lookup(flow_table, &tuple);
if (tuplehash == NULL)
return NF_ACCEPT;
@@ -272,7 +306,14 @@ static void nf_flow_inet_xmit(struct flow_offload *flow, struct sk_buff *skb,
flow->timeout = (u32)jiffies + NF_FLOW_TIMEOUT;
- nf_flow_inet_xmit(flow, skb, dir);
+ switch (family) {
+ case NFPROTO_IPV4:
+ nf_flow_inet_xmit(flow, skb, dir);
+ break;
+ case NFPROTO_BRIDGE:
+ nf_flow_bridge_xmit(flow, skb, dir);
+ break;
+ }
return NF_STOLEN;
}
diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
index 4af94ce..0cdadd4 100644
--- a/net/netfilter/nft_flow_offload.c
+++ b/net/netfilter/nft_flow_offload.c
@@ -14,6 +14,10 @@
#include <linux/netfilter/nf_conntrack_common.h>
#include <net/netfilter/nf_flow_table.h>
+#ifdef CONFIG_NF_TABLES_BRIDGE
+#include "../bridge/br_private.h"
+#endif
+
struct nft_flow_offload {
struct nft_flowtable *flowtable;
};
@@ -49,23 +53,130 @@ static int nft_flow_route(const struct nft_pktinfo *pkt,
return 0;
}
+static int nft_flow_forward(const struct nft_pktinfo *pkt,
+ const struct nf_conn *ct,
+ struct nf_flow_forward *forward,
+ enum ip_conntrack_dir dir)
+{
+#ifdef CONFIG_NF_TABLES_BRIDGE
+ struct net_bridge_vlan_group *vg;
+ const struct net_bridge_port *p;
+ u16 vlan_proto = 0;
+ u16 vid = 0;
+
+ if (skb_vlan_tag_present(pkt->skb)) {
+ vid = skb_vlan_tag_get_id(pkt->skb);
+ vlan_proto = pkt->skb->vlan_proto;
+ }
+
+ forward->tuple[dir].dst_port.dst_vlan_tag = vid;
+ forward->tuple[dir].dst_port.vlan_proto = vlan_proto;
+ forward->tuple[!dir].vlan_tag = vid;
+ forward->tuple[dir].dst_port.dev = dev_get_by_index(dev_net(nft_out(pkt)),
+ nft_out(pkt)->ifindex);
+ forward->tuple[!dir].dst_port.dev = dev_get_by_index(dev_net(nft_in(pkt)),
+ nft_in(pkt)->ifindex);
+
+ p = br_port_get_rtnl_rcu(nft_out(pkt));
+ if (!p)
+ goto err;
+ if (!br_opt_get(p->br, BROPT_VLAN_ENABLED))
+ goto out;
+
+ if (!vid) {
+ vg = nbp_vlan_group_rcu(p);
+ vid = br_get_pvid(vg);
+ }
+
+ if (vid) {
+ struct net_bridge_vlan *vlan;
+
+ p = br_port_get_rtnl_rcu(nft_in(pkt));
+ if (!p)
+ goto err;
+
+ vlan_proto = p->br->vlan_proto;
+ vg = nbp_vlan_group_rcu(p);
+ vlan = br_vlan_find(vg, vid);
+ if (vlan && vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED) {
+ vid = 0;
+ vlan_proto = 0;
+ }
+ }
+
+out:
+ forward->tuple[!dir].dst_port.vlan_proto = vlan_proto;
+ forward->tuple[!dir].dst_port.dst_vlan_tag = vid;
+ forward->tuple[dir].vlan_tag = vid;
+
+ return 0;
+
+err:
+ dev_put(forward->tuple[dir].dst_port.dev);
+ dev_put(forward->tuple[!dir].dst_port.dev);
+#endif
+ return -ENOENT;
+}
+
static bool nft_flow_offload_skip(struct sk_buff *skb, int family)
{
if (skb_sec_path(skb))
return true;
- if (family == NFPROTO_IPV4) {
+ switch (family) {
+ case NFPROTO_IPV4: {
const struct ip_options *opt;
opt = &(IPCB(skb)->opt);
if (unlikely(opt->optlen))
return true;
+ break;
+ }
+ case NFPROTO_BRIDGE: {
+ const struct iphdr *iph;
+
+ if (skb->protocol != htons(ETH_P_IP))
+ return true;
+
+ iph = ip_hdr(skb);
+ if (iph->ihl > 5)
+ return true;
+ break;
+ }
}
return false;
}
+static void flow_offload_release_dst(struct nf_flow_dst *flow_dst,
+ enum ip_conntrack_dir dir)
+{
+ if (flow_dst->type == FLOW_OFFLOAD_TYPE_BRIDGE) {
+ dev_put(flow_dst->forward.tuple[dir].dst_port.dev);
+ dev_put(flow_dst->forward.tuple[!dir].dst_port.dev);
+ } else {
+ dst_release(flow_dst->route.tuple[!dir].dst);
+ }
+}
+
+static int flow_offload_get_dst(const struct nft_pktinfo *pkt, struct nf_conn *ct,
+ enum ip_conntrack_dir dir, int family,
+ struct nf_flow_dst *flow_dst)
+{
+ if (family == NFPROTO_BRIDGE) {
+ flow_dst->type = FLOW_OFFLOAD_TYPE_BRIDGE;
+ if (nft_flow_forward(pkt, ct, &flow_dst->forward, dir) < 0)
+ return -1;
+ } else {
+ flow_dst->type = FLOW_OFFLOAD_TYPE_INET;
+ if (nft_flow_route(pkt, ct, &flow_dst->route, dir) < 0)
+ return -1;
+ }
+
+ return 0;
+}
+
static void nft_flow_offload_eval(const struct nft_expr *expr,
struct nft_regs *regs,
const struct nft_pktinfo *pkt)
@@ -76,11 +187,12 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
struct nf_flow_dst flow_dst;
struct flow_offload *flow;
enum ip_conntrack_dir dir;
+ int family = nft_pf(pkt);
bool is_tcp = false;
struct nf_conn *ct;
int ret;
- if (nft_flow_offload_skip(pkt->skb, nft_pf(pkt)))
+ if (nft_flow_offload_skip(pkt->skb, family))
goto out;
ct = nf_ct_get(pkt->skb, &ctinfo);
@@ -108,8 +220,9 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
goto out;
dir = CTINFO2DIR(ctinfo);
- if (nft_flow_route(pkt, ct, &flow_dst.route, dir) < 0)
- goto err_flow_route;
+
+ if (flow_offload_get_dst(pkt, ct, dir, family, &flow_dst) < 0)
+ goto err_flow_dst;
flow = flow_offload_alloc(ct, &flow_dst);
if (!flow)
@@ -124,14 +237,16 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
if (ret < 0)
goto err_flow_add;
- dst_release(flow_dst.route.tuple[!dir].dst);
+ if (family != NFPROTO_BRIDGE)
+ dst_release(flow_dst.route.tuple[!dir].dst);
+
return;
err_flow_add:
flow_offload_free(flow);
err_flow_alloc:
- dst_release(flow_dst.route.tuple[!dir].dst);
-err_flow_route:
+ flow_offload_release_dst(&flow_dst, dir);
+err_flow_dst:
clear_bit(IPS_OFFLOAD_BIT, &ct->status);
out:
regs->verdict.code = NFT_BREAK;
--
1.8.3.1
^ permalink raw reply related
* [PATCH 4/4 nf-next v2] netfilter: Flow table support for the bridge family
From: wenxu @ 2019-06-28 9:48 UTC (permalink / raw)
To: pablo, fw; +Cc: netfilter-devel, netdev
In-Reply-To: <1561715326-3945-1-git-send-email-wenxu@ucloud.cn>
From: wenxu <wenxu@ucloud.cn>
This patch adds the bridge flow table type, that implements the datapath
flow table to forward IPv4 traffic through bridge.
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
net/bridge/netfilter/Kconfig | 8 +++++
net/bridge/netfilter/Makefile | 1 +
net/bridge/netfilter/nf_flow_table_bridge.c | 46 +++++++++++++++++++++++++++++
3 files changed, 55 insertions(+)
create mode 100644 net/bridge/netfilter/nf_flow_table_bridge.c
diff --git a/net/bridge/netfilter/Kconfig b/net/bridge/netfilter/Kconfig
index f4fb0b9..cba5f71 100644
--- a/net/bridge/netfilter/Kconfig
+++ b/net/bridge/netfilter/Kconfig
@@ -33,6 +33,14 @@ config NF_CONNTRACK_BRIDGE
To compile it as a module, choose M here. If unsure, say N.
+config NF_FLOW_TABLE_BRIDGE
+ tristate "Netfilter flow table bridge module"
+ depends on NF_FLOW_TABLE && NF_CONNTRACK_BRIDGE
+ help
+ This option adds the flow table bridge support.
+
+ To compile it as a module, choose M here.
+
endif # NF_TABLES_BRIDGE
menuconfig BRIDGE_NF_EBTABLES
diff --git a/net/bridge/netfilter/Makefile b/net/bridge/netfilter/Makefile
index 9d77673..deb81e6 100644
--- a/net/bridge/netfilter/Makefile
+++ b/net/bridge/netfilter/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_NFT_BRIDGE_REJECT) += nft_reject_bridge.o
# connection tracking
obj-$(CONFIG_NF_CONNTRACK_BRIDGE) += nf_conntrack_bridge.o
+obj-$(CONFIG_NF_FLOW_TABLE_BRIDGE) += nf_flow_table_bridge.o
# packet logging
obj-$(CONFIG_NF_LOG_BRIDGE) += nf_log_bridge.o
diff --git a/net/bridge/netfilter/nf_flow_table_bridge.c b/net/bridge/netfilter/nf_flow_table_bridge.c
new file mode 100644
index 0000000..3a65b44
--- /dev/null
+++ b/net/bridge/netfilter/nf_flow_table_bridge.c
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/netfilter.h>
+#include <net/netfilter/nf_flow_table.h>
+#include <net/netfilter/nf_tables.h>
+
+static unsigned int
+nf_flow_offload_bridge_hook(void *priv, struct sk_buff *skb,
+ const struct nf_hook_state *state)
+{
+ switch (skb->protocol) {
+ case htons(ETH_P_IP):
+ return nf_flow_offload_ip_hook(priv, skb, state);
+ }
+
+ return NF_ACCEPT;
+}
+
+static struct nf_flowtable_type flowtable_bridge = {
+ .family = NFPROTO_BRIDGE,
+ .init = nf_flow_table_init,
+ .free = nf_flow_table_free,
+ .hook = nf_flow_offload_bridge_hook,
+ .owner = THIS_MODULE,
+};
+
+static int __init nf_flow_bridge_module_init(void)
+{
+ nft_register_flowtable_type(&flowtable_bridge);
+
+ return 0;
+}
+
+static void __exit nf_flow_bridge_module_exit(void)
+{
+ nft_unregister_flowtable_type(&flowtable_bridge);
+}
+
+module_init(nf_flow_bridge_module_init);
+module_exit(nf_flow_bridge_module_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("wenxu <wenxu@ucloud.cn>");
+MODULE_ALIAS_NF_FLOWTABLE(AF_BRIDGE);
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 2/3 nf-next] netfilter:nf_flow_table: Support bridge type flow offload
From: wenxu @ 2019-06-28 9:51 UTC (permalink / raw)
To: Florian Westphal; +Cc: Pablo Neira Ayuso, netfilter-devel, netdev
In-Reply-To: <20190628060617.az2quv4bodrenuli@breakpoint.cc>
On 6/28/2019 2:06 PM, Florian Westphal wrote:
> wenxu <wenxu@ucloud.cn> wrote:
>> ns21 iperf to 10.0.0.8 with dport 22 in ns22
>> first time with OFFLOAD enable
>>
>> nft add flowtable bridge firewall fb2 { hook ingress priority 0 \; devices = { veth21, veth22 } \; }
>> nft add chain bridge firewall ftb-all {type filter hook forward priority 0 \; policy accept \; }
>> nft add rule bridge firewall ftb-all counter ct zone 2 ip protocol tcp flow offload @fb2
>>
>> # iperf -c 10.0.0.8 -p 22 -t 60 -i2
> [..]
>> [ 3] 0.0-60.0 sec 353 GBytes 50.5 Gbits/sec
>>
>> The second time on any offload:
>> # iperf -c 10.0.0.8 -p 22 -t 60 -i2
>> [ 3] 0.0-60.0 sec 271 GBytes 38.8 Gbits/sec
> Wow, this is pretty impressive. Do you have numbers without
> offload and no connection tracking?
There is no other connection on the bridge in zone 2
>
> Is this with CONFIG_RETPOLINE=y (just curious)?
Yes, it is enable.
^ permalink raw reply
* [PATCH iproute2-next v4 1/2] ipaddress: correctly print a VF hw address in the IPoIB case
From: Denis Kirjanov @ 2019-06-28 9:54 UTC (permalink / raw)
To: stephen, dsahern; +Cc: netdev, linux-rdma, dledford, mkubecek, Denis Kirjanov
Current code assumes that we print ethernet mac and
that doesn't work in the IPoIB case with SRIOV-enabled hardware
Before:
11: ib1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 2044 qdisc pfifo_fast
state UP mode DEFAULT group default qlen 256
link/infiniband
80:00:00:66:fe:80:00:00:00:00:00:00:24:8a:07:03:00:a4:3e:7c brd
00:ff:ff:ff:ff:12:40:1b:ff:ff:00:00:00:00:00:00:ff:ff:ff:ff
vf 0 MAC 14:80:00:00:66:fe, spoof checking off, link-state
disable,
trust off, query_rss off
...
After:
11: ib1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 2044 qdisc pfifo_fast
state UP mode DEFAULT group default qlen 256
link/infiniband
80:00:00:66:fe:80:00:00:00:00:00:00:24:8a:07:03:00:a4:3e:7c brd
00:ff:ff:ff:ff:12:40:1b:ff:ff:00:00:00:00:00:00:ff:ff:ff:ff
vf 0 link/infiniband
80:00:00:66:fe:80:00:00:00:00:00:00:24:8a:07:03:00:a4:3e:7c brd
00:ff:ff:ff:ff:12:40:1b:ff:ff:00:00:00:00:00:00:ff:ff:ff:ff, spoof
checking off, link-state disable, trust off, query_rss off
v1->v2: updated kernel headers to uapi commit
v2->v3: fixed alignment
v3->v4: aligned print statements as used through the source
Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
---
ip/ipaddress.c | 40 +++++++++++++++++++++++++++++++++++-----
1 file changed, 35 insertions(+), 5 deletions(-)
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index b504200b..741b8667 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -26,6 +26,7 @@
#include <linux/netdevice.h>
#include <linux/if_arp.h>
+#include <linux/if_infiniband.h>
#include <linux/sockios.h>
#include <linux/net_namespace.h>
@@ -349,9 +350,10 @@ static void print_af_spec(FILE *fp, struct rtattr *af_spec_attr)
static void print_vf_stats64(FILE *fp, struct rtattr *vfstats);
-static void print_vfinfo(FILE *fp, struct rtattr *vfinfo)
+static void print_vfinfo(struct ifinfomsg *ifi, FILE *fp, struct rtattr *vfinfo)
{
struct ifla_vf_mac *vf_mac;
+ struct ifla_vf_broadcast *vf_broadcast;
struct ifla_vf_tx_rate *vf_tx_rate;
struct rtattr *vf[IFLA_VF_MAX + 1] = {};
@@ -365,13 +367,41 @@ static void print_vfinfo(FILE *fp, struct rtattr *vfinfo)
parse_rtattr_nested(vf, IFLA_VF_MAX, vfinfo);
vf_mac = RTA_DATA(vf[IFLA_VF_MAC]);
+ vf_broadcast = RTA_DATA(vf[IFLA_VF_BROADCAST]);
vf_tx_rate = RTA_DATA(vf[IFLA_VF_TX_RATE]);
print_string(PRINT_FP, NULL, "%s ", _SL_);
print_int(PRINT_ANY, "vf", "vf %d ", vf_mac->vf);
- print_string(PRINT_ANY, "mac", "MAC %s",
- ll_addr_n2a((unsigned char *) &vf_mac->mac,
- ETH_ALEN, 0, b1, sizeof(b1)));
+
+ print_string(PRINT_ANY,
+ "link_type",
+ " link/%s ",
+ ll_type_n2a(ifi->ifi_type, b1, sizeof(b1)));
+
+ print_color_string(PRINT_ANY, COLOR_MAC,
+ "address", "%s",
+ ll_addr_n2a((unsigned char *) &vf_mac->mac,
+ ifi->ifi_type == ARPHRD_ETHER ?
+ ETH_ALEN : INFINIBAND_ALEN,
+ ifi->ifi_type,
+ b1, sizeof(b1)));
+
+ if (vf[IFLA_VF_BROADCAST]) {
+ if (ifi->ifi_flags&IFF_POINTOPOINT) {
+ print_string(PRINT_FP, NULL, " peer ", NULL);
+ print_bool(PRINT_JSON,
+ "link_pointtopoint", NULL, true);
+ } else
+ print_string(PRINT_FP, NULL, " brd ", NULL);
+
+ print_color_string(PRINT_ANY, COLOR_MAC,
+ "broadcast", "%s",
+ ll_addr_n2a((unsigned char *) &vf_broadcast->broadcast,
+ ifi->ifi_type == ARPHRD_ETHER ?
+ ETH_ALEN : INFINIBAND_ALEN,
+ ifi->ifi_type,
+ b1, sizeof(b1)));
+ }
if (vf[IFLA_VF_VLAN_LIST]) {
struct rtattr *i, *vfvlanlist = vf[IFLA_VF_VLAN_LIST];
@@ -1102,7 +1132,7 @@ int print_linkinfo(struct nlmsghdr *n, void *arg)
open_json_array(PRINT_JSON, "vfinfo_list");
for (i = RTA_DATA(vflist); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) {
open_json_object(NULL);
- print_vfinfo(fp, i);
+ print_vfinfo(ifi, fp, i);
close_json_object();
}
close_json_array(PRINT_JSON, NULL);
--
2.12.3
^ permalink raw reply related
* [PATCH iproute2-next v4 2/2] uapi: update if_link.h
From: Denis Kirjanov @ 2019-06-28 9:54 UTC (permalink / raw)
To: stephen, dsahern; +Cc: netdev, linux-rdma, dledford, mkubecek, Denis Kirjanov
In-Reply-To: <20190628095426.2819-1-dkirjanov@suse.com>
update if_link.h to commit 75345f888f700c4ab2448287e35d48c760b202e6
("ipoib: show VF broadcast address")
Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
---
include/uapi/linux/if_link.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index bfe7f9e6..5f271d84 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -692,6 +692,7 @@ enum {
IFLA_VF_IB_NODE_GUID, /* VF Infiniband node GUID */
IFLA_VF_IB_PORT_GUID, /* VF Infiniband port GUID */
IFLA_VF_VLAN_LIST, /* nested list of vlans, option for QinQ */
+ IFLA_VF_BROADCAST, /* VF broadcast */
__IFLA_VF_MAX,
};
@@ -702,6 +703,10 @@ struct ifla_vf_mac {
__u8 mac[32]; /* MAX_ADDR_LEN */
};
+struct ifla_vf_broadcast {
+ __u8 broadcast[32];
+};
+
struct ifla_vf_vlan {
__u32 vf;
__u32 vlan; /* 0 - 4095, 0 disables VLAN filter */
--
2.12.3
^ permalink raw reply related
* Re: [PATCH v2 net-next 0/3] Better PHYLINK compliance for SJA1105 DSA
From: Russell King - ARM Linux admin @ 2019-06-28 10:05 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: f.fainelli, vivien.didelot, andrew, davem, netdev
In-Reply-To: <20190627214637.22366-1-olteanv@gmail.com>
On Fri, Jun 28, 2019 at 12:46:34AM +0300, Vladimir Oltean wrote:
> After discussing with Russell King, it appears this driver is making a
> few confusions and not performing some checks for consistent operation.
>
> Changes in v2:
> - Removed redundant print in the phylink_validate callback (in 2/3).
>
> Vladimir Oltean (3):
> net: dsa: sja1105: Don't check state->link in phylink_mac_config
> net: dsa: sja1105: Check for PHY mode mismatches with what PHYLINK
> reports
> net: dsa: sja1105: Mark in-band AN modes not supported for PHYLINK
>
> drivers/net/dsa/sja1105/sja1105_main.c | 56 +++++++++++++++++++++++++-
> 1 file changed, 54 insertions(+), 2 deletions(-)
Thanks. For the whole series:
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* Re: [PATCH bpf-next v2] libbpf: add xsk_ring_prod__nb_free() function
From: Magnus Karlsson @ 2019-06-28 10:14 UTC (permalink / raw)
To: Eelco Chaudron
Cc: Network Development, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko
In-Reply-To: <d4692ea57ba7a3fe33549fc6222fb8aea5a4225e.1561537968.git.echaudro@redhat.com>
On Wed, Jun 26, 2019 at 10:33 AM Eelco Chaudron <echaudro@redhat.com> wrote:
>
> When an AF_XDP application received X packets, it does not mean X
> frames can be stuffed into the producer ring. To make it easier for
> AF_XDP applications this API allows them to check how many frames can
> be added into the ring.
>
> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> ---
>
> v1 -> v2
> - Renamed xsk_ring_prod__free() to xsk_ring_prod__nb_free()
> - Add caching so it will only touch global state when needed
>
> tools/lib/bpf/xsk.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
> index 82ea71a0f3ec..6acb81102346 100644
> --- a/tools/lib/bpf/xsk.h
> +++ b/tools/lib/bpf/xsk.h
> @@ -76,11 +76,11 @@ xsk_ring_cons__rx_desc(const struct xsk_ring_cons *rx, __u32 idx)
> return &descs[idx & rx->mask];
> }
>
> -static inline __u32 xsk_prod_nb_free(struct xsk_ring_prod *r, __u32 nb)
> +static inline __u32 xsk_prod__nb_free(struct xsk_ring_prod *r, __u32 nb)
> {
> __u32 free_entries = r->cached_cons - r->cached_prod;
>
> - if (free_entries >= nb)
> + if (free_entries >= nb && nb != 0)
> return free_entries;
Thanks Eelco for the patch. Is the test nb != 0 introduced here so
that the function will continue with the refresh from the global state
when nb is set to 0? If so, could a user not instead just set the nb
parameter to the size of the ring? This would always trigger a
refresh, except when the number of free entries is equal to the size
of the ring, but then we do not need the refresh anyway. This would
eliminate the nb != 0 test that you introduced from the fast path.
/Magnus
> /* Refresh the local tail pointer.
> @@ -110,7 +110,7 @@ static inline __u32 xsk_cons_nb_avail(struct xsk_ring_cons *r, __u32 nb)
> static inline size_t xsk_ring_prod__reserve(struct xsk_ring_prod *prod,
> size_t nb, __u32 *idx)
> {
> - if (xsk_prod_nb_free(prod, nb) < nb)
> + if (xsk_prod__nb_free(prod, nb) < nb)
> return 0;
>
> *idx = prod->cached_prod;
> --
> 2.20.1
>
^ permalink raw reply
* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Christian Brauner @ 2019-06-28 10:28 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Kees Cook, Linux API, Song Liu, Network Development, bpf,
Alexei Starovoitov, Daniel Borkmann, kernel-team, lmb, Jann Horn,
Greg KH, casey, sds, linux-security
In-Reply-To: <CALCETrUp3Tj062wG-noNdsY-sU9gsob_kVK=W_DxWciMpZFvyA@mail.gmail.com>
On Thu, Jun 27, 2019 at 04:42:18PM -0700, Andy Lutomirski wrote:
> [sigh, I finally set up lore nntp, and I goofed some addresses. Hi
> Kees and linux-api.]
Love it or hate it but that should probably also Cc linux-security...
>
> On Thu, Jun 27, 2019 at 4:40 PM Andy Lutomirski <luto@kernel.org> wrote:
> >
> > On 6/27/19 1:19 PM, Song Liu wrote:
> > > This patch introduce unprivileged BPF access. The access control is
> > > achieved via device /dev/bpf. Users with write access to /dev/bpf are able
> > > to call sys_bpf().
> > >
> > > Two ioctl command are added to /dev/bpf:
> > >
> > > The two commands enable/disable permission to call sys_bpf() for current
> > > task. This permission is noted by bpf_permitted in task_struct. This
> > > permission is inherited during clone(CLONE_THREAD).
> > >
> > > Helper function bpf_capable() is added to check whether the task has got
> > > permission via /dev/bpf.
> > >
> >
> > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > > index 0e079b2298f8..79dc4d641cf3 100644
> > > --- a/kernel/bpf/verifier.c
> > > +++ b/kernel/bpf/verifier.c
> > > @@ -9134,7 +9134,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr,
> > > env->insn_aux_data[i].orig_idx = i;
> > > env->prog = *prog;
> > > env->ops = bpf_verifier_ops[env->prog->type];
> > > - is_priv = capable(CAP_SYS_ADMIN);
> > > + is_priv = bpf_capable(CAP_SYS_ADMIN);
> >
> > Huh? This isn't a hardening measure -- the "is_priv" verifier mode
> > allows straight-up leaks of private kernel state to user mode.
> >
> > (For that matter, the pending lockdown stuff should possibly consider
> > this a "confidentiality" issue.)
> >
> >
> > I have a bigger issue with this patch, though: it's a really awkward way
> > to pretend to have capabilities. For bpf, it seems like you could make
> > this be a *real* capability without too much pain since there's only one
> > syscall there. Just find a way to pass an fd to /dev/bpf into the
> > syscall. If this means you need a new bpf_with_cap() syscall that takes
> > an extra argument, so be it. The old bpf() syscall can just translate
> > to bpf_with_cap(..., -1).
> >
> > For a while, I've considered a scheme I call "implicit rights". There
> > would be a directory in /dev called /dev/implicit_rights. This would
> > either be part of devtmpfs or a whole new filesystem -- it would *not*
> > be any other filesystem. The contents would be files that can't be read
> > or written and exist only in memory. You create them with a privileged
> > syscall. Certain actions that are sensitive but not at the level of
> > CAP_SYS_ADMIN (use of large-attack-surface bpf stuff, creation of user
> > namespaces, profiling the kernel, etc) could require an "implicit
> > right". When you do them, if you don't have CAP_SYS_ADMIN, the kernel
> > would do a path walk for, say, /dev/implicit_rights/bpf and, if the
> > object exists, can be opened, and actually refers to the "bpf" rights
> > object, then the action is allowed. Otherwise it's denied.
> >
> > This is extensible, and it doesn't require the rather ugly per-task
> > state of whether it's enabled.
> >
> > For things like creation of user namespaces, there's an existing API,
> > and the default is that it works without privilege. Switching it to an
> > implicit right has the benefit of not requiring code changes to programs
> > that already work as non-root.
> >
> > But, for BPF in particular, this type of compatibility issue doesn't
> > exist now. You already can't use most eBPF functionality without
> > privilege. New bpf-using programs meant to run without privilege are
> > *new*, so they can use a new improved API. So, rather than adding this
> > obnoxious ioctl, just make the API explicit, please.
> >
> > Also, please cc: linux-abi next time.
^ permalink raw reply
* [PATCH] hinic: reduce rss_init stack usage
From: Arnd Bergmann @ 2019-06-28 10:31 UTC (permalink / raw)
To: Aviad Krawczyk, David S. Miller
Cc: Arnd Bergmann, Xue Chaojing, Jesse Brandeburg, Zhao Chen,
Eric Dumazet, dann frazier, netdev, linux-kernel
On 32-bit architectures, putting an array of 256 u32 values on the
stack uses more space than the warning limit:
drivers/net/ethernet/huawei/hinic/hinic_main.c: In function 'hinic_rss_init':
drivers/net/ethernet/huawei/hinic/hinic_main.c:286:1: error: the frame size of 1068 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
I considered changing the code to use u8 values here, since that's
all the hardware supports, but dynamically allocating the array is
a more isolated fix here.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
.../net/ethernet/huawei/hinic/hinic_main.c | 20 ++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_main.c b/drivers/net/ethernet/huawei/hinic/hinic_main.c
index 1b917543feac..ceb0e247f52d 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_main.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_main.c
@@ -256,37 +256,43 @@ static int hinic_configure_max_qnum(struct hinic_dev *nic_dev)
static int hinic_rss_init(struct hinic_dev *nic_dev)
{
- u32 indir_tbl[HINIC_RSS_INDIR_SIZE] = { 0 };
u8 default_rss_key[HINIC_RSS_KEY_SIZE];
u8 tmpl_idx = nic_dev->rss_tmpl_idx;
+ u32 *indir_tbl;
int err, i;
+ indir_tbl = kcalloc(HINIC_RSS_INDIR_SIZE, sizeof(u32), GFP_KERNEL);
+ if (!indir_tbl)
+ return -ENOMEM;
+
netdev_rss_key_fill(default_rss_key, sizeof(default_rss_key));
for (i = 0; i < HINIC_RSS_INDIR_SIZE; i++)
indir_tbl[i] = ethtool_rxfh_indir_default(i, nic_dev->num_rss);
err = hinic_rss_set_template_tbl(nic_dev, tmpl_idx, default_rss_key);
if (err)
- return err;
+ goto out;
err = hinic_rss_set_indir_tbl(nic_dev, tmpl_idx, indir_tbl);
if (err)
- return err;
+ goto out;
err = hinic_set_rss_type(nic_dev, tmpl_idx, nic_dev->rss_type);
if (err)
- return err;
+ goto out;
err = hinic_rss_set_hash_engine(nic_dev, tmpl_idx,
nic_dev->rss_hash_engine);
if (err)
- return err;
+ goto out;
err = hinic_rss_cfg(nic_dev, 1, tmpl_idx);
if (err)
- return err;
+ goto out;
- return 0;
+out:
+ kfree(indir_tbl);
+ return err;
}
static void hinic_rss_deinit(struct hinic_dev *nic_dev)
--
2.20.0
^ permalink raw reply related
* [PATCH 0/3, net-next] net: netsec: Add XDP Support
From: Ilias Apalodimas @ 2019-06-28 10:39 UTC (permalink / raw)
To: netdev, jaswinder.singh
Cc: ard.biesheuvel, bjorn.topel, magnus.karlsson, brouer, daniel, ast,
makita.toshiaki, jakub.kicinski, john.fastabend, davem,
maciejromanfijalkowski, Ilias Apalodimas
This is a respin of https://www.spinics.net/lists/netdev/msg526066.html
Since page_pool API fixes are merged into net-next we can now safely use
it's DMA mapping capabilities.
First patch changes the buffer allocation from napi/netdev_alloc_frag()
to page_pool API. Although this will lead to slightly reduced performance
(on raw packet drops only) we can use the API for XDP buffer recycling.
Another side effect is a slight increase in memory usage, due to using a
single page per packet.
The second patch adds XDP support on the driver.
There's a bunch of interesting options that come up due to the single
Tx queue.
Locking is needed(to avoid messing up the Tx queues since ndo_xdp_xmit
and the normal stack can co-exist). We also need to track down the
'buffer type' for TX and properly free or recycle the packet depending
on it's nature.
Changes since RFC:
- Bug fixes from Jesper and Maciej
- Added page pool API to retrieve the DMA direction
Ilias Apalodimas (3):
net: netsec: Use page_pool API
net: page_pool: add helper function for retrieving dma direction
net: netsec: add XDP support
drivers/net/ethernet/socionext/Kconfig | 1 +
drivers/net/ethernet/socionext/netsec.c | 469 ++++++++++++++++++++----
include/net/page_pool.h | 9 +
3 files changed, 412 insertions(+), 67 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH 1/3, net-next] net: netsec: Use page_pool API
From: Ilias Apalodimas @ 2019-06-28 10:39 UTC (permalink / raw)
To: netdev, jaswinder.singh
Cc: ard.biesheuvel, bjorn.topel, magnus.karlsson, brouer, daniel, ast,
makita.toshiaki, jakub.kicinski, john.fastabend, davem,
maciejromanfijalkowski, Ilias Apalodimas
In-Reply-To: <1561718355-13919-1-git-send-email-ilias.apalodimas@linaro.org>
Use page_pool and it's DMA mapping capabilities for Rx buffers instead
of netdev/napi_alloc_frag()
Although this will result in a slight performance penalty on small sized
packets (~10%) the use of the API will allow to easily add XDP support.
The penalty won't be visible in network testing i.e ipef/netperf etc, it
only happens during raw packet drops.
Furthermore we intend to add recycling capabilities on the API
in the future. Once the recycling is added the performance penalty will
go away.
The only 'real' penalty is the slightly increased memory usage, since we
now allocate a page per packet instead of the amount of bytes we need +
skb metadata (difference is roughly 2kb per packet).
With a minimum of 4BG of RAM on the only SoC that has this NIC the
extra memory usage is negligible (a bit more on 64K pages)
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
drivers/net/ethernet/socionext/Kconfig | 1 +
drivers/net/ethernet/socionext/netsec.c | 121 +++++++++++++++---------
2 files changed, 75 insertions(+), 47 deletions(-)
diff --git a/drivers/net/ethernet/socionext/Kconfig b/drivers/net/ethernet/socionext/Kconfig
index 25f18be27423..95e99baf3f45 100644
--- a/drivers/net/ethernet/socionext/Kconfig
+++ b/drivers/net/ethernet/socionext/Kconfig
@@ -26,6 +26,7 @@ config SNI_NETSEC
tristate "Socionext NETSEC ethernet support"
depends on (ARCH_SYNQUACER || COMPILE_TEST) && OF
select PHYLIB
+ select PAGE_POOL
select MII
---help---
Enable to add support for the SocioNext NetSec Gigabit Ethernet
diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
index 48fd7448b513..e653b24d0534 100644
--- a/drivers/net/ethernet/socionext/netsec.c
+++ b/drivers/net/ethernet/socionext/netsec.c
@@ -11,6 +11,7 @@
#include <linux/io.h>
#include <net/tcp.h>
+#include <net/page_pool.h>
#include <net/ip6_checksum.h>
#define NETSEC_REG_SOFT_RST 0x104
@@ -235,7 +236,8 @@
#define DESC_NUM 256
#define NETSEC_SKB_PAD (NET_SKB_PAD + NET_IP_ALIGN)
-#define NETSEC_RX_BUF_SZ 1536
+#define NETSEC_RX_BUF_NON_DATA (NETSEC_SKB_PAD + \
+ SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
#define DESC_SZ sizeof(struct netsec_de)
@@ -258,6 +260,8 @@ struct netsec_desc_ring {
struct netsec_desc *desc;
void *vaddr;
u16 head, tail;
+ struct page_pool *page_pool;
+ struct xdp_rxq_info xdp_rxq;
};
struct netsec_priv {
@@ -673,33 +677,27 @@ static void netsec_process_tx(struct netsec_priv *priv)
}
static void *netsec_alloc_rx_data(struct netsec_priv *priv,
- dma_addr_t *dma_handle, u16 *desc_len,
- bool napi)
+ dma_addr_t *dma_handle, u16 *desc_len)
+
{
- size_t total_len = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
- size_t payload_len = NETSEC_RX_BUF_SZ;
- dma_addr_t mapping;
- void *buf;
- total_len += SKB_DATA_ALIGN(payload_len + NETSEC_SKB_PAD);
+ struct netsec_desc_ring *dring = &priv->desc_ring[NETSEC_RING_RX];
+ struct page *page;
- buf = napi ? napi_alloc_frag(total_len) : netdev_alloc_frag(total_len);
- if (!buf)
+ page = page_pool_dev_alloc_pages(dring->page_pool);
+ if (!page)
return NULL;
- mapping = dma_map_single(priv->dev, buf + NETSEC_SKB_PAD, payload_len,
- DMA_FROM_DEVICE);
- if (unlikely(dma_mapping_error(priv->dev, mapping)))
- goto err_out;
-
- *dma_handle = mapping;
- *desc_len = payload_len;
-
- return buf;
+ /* page_pool API will map the whole page, skip
+ * NET_SKB_PAD + NET_IP_ALIGN for the payload
+ */
+ *dma_handle = page_pool_get_dma_addr(page) + NETSEC_SKB_PAD;
+ /* make sure the incoming payload fits in the page with the needed
+ * NET_SKB_PAD + NET_IP_ALIGN + skb_shared_info
+ */
+ *desc_len = PAGE_SIZE - NETSEC_RX_BUF_NON_DATA;
-err_out:
- skb_free_frag(buf);
- return NULL;
+ return page_address(page);
}
static void netsec_rx_fill(struct netsec_priv *priv, u16 from, u16 num)
@@ -728,10 +726,10 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget)
u16 idx = dring->tail;
struct netsec_de *de = dring->vaddr + (DESC_SZ * idx);
struct netsec_desc *desc = &dring->desc[idx];
+ struct page *page = virt_to_page(desc->addr);
u16 pkt_len, desc_len;
dma_addr_t dma_handle;
void *buf_addr;
- u32 truesize;
if (de->attr & (1U << NETSEC_RX_PKT_OWN_FIELD)) {
/* reading the register clears the irq */
@@ -766,8 +764,8 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget)
/* allocate a fresh buffer and map it to the hardware.
* This will eventually replace the old buffer in the hardware
*/
- buf_addr = netsec_alloc_rx_data(priv, &dma_handle, &desc_len,
- true);
+ buf_addr = netsec_alloc_rx_data(priv, &dma_handle, &desc_len);
+
if (unlikely(!buf_addr))
break;
@@ -775,22 +773,19 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget)
DMA_FROM_DEVICE);
prefetch(desc->addr);
- truesize = SKB_DATA_ALIGN(desc->len + NETSEC_SKB_PAD) +
- SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
- skb = build_skb(desc->addr, truesize);
+ skb = build_skb(desc->addr, desc->len + NETSEC_RX_BUF_NON_DATA);
if (unlikely(!skb)) {
- /* free the newly allocated buffer, we are not going to
- * use it
+ /* If skb fails recycle_direct will either unmap and
+ * free the page or refill the cache depending on the
+ * cache state. Since we paid the allocation cost if
+ * building an skb fails try to put the page into cache
*/
- dma_unmap_single(priv->dev, dma_handle, desc_len,
- DMA_FROM_DEVICE);
- skb_free_frag(buf_addr);
+ page_pool_recycle_direct(dring->page_pool, page);
netif_err(priv, drv, priv->ndev,
"rx failed to build skb\n");
break;
}
- dma_unmap_single_attrs(priv->dev, desc->dma_addr, desc->len,
- DMA_FROM_DEVICE, DMA_ATTR_SKIP_CPU_SYNC);
+ page_pool_release_page(dring->page_pool, page);
/* Update the descriptor with the new buffer we allocated */
desc->len = desc_len;
@@ -980,21 +975,26 @@ static void netsec_uninit_pkt_dring(struct netsec_priv *priv, int id)
if (!dring->vaddr || !dring->desc)
return;
-
for (idx = 0; idx < DESC_NUM; idx++) {
desc = &dring->desc[idx];
if (!desc->addr)
continue;
- dma_unmap_single(priv->dev, desc->dma_addr, desc->len,
- id == NETSEC_RING_RX ? DMA_FROM_DEVICE :
- DMA_TO_DEVICE);
- if (id == NETSEC_RING_RX)
- skb_free_frag(desc->addr);
- else if (id == NETSEC_RING_TX)
+ if (id == NETSEC_RING_RX) {
+ struct page *page = virt_to_page(desc->addr);
+
+ page_pool_put_page(dring->page_pool, page, false);
+ } else if (id == NETSEC_RING_TX) {
+ dma_unmap_single(priv->dev, desc->dma_addr, desc->len,
+ DMA_TO_DEVICE);
dev_kfree_skb(desc->skb);
+ }
}
+ /* Rx is currently using page_pool */
+ if (xdp_rxq_info_is_reg(&dring->xdp_rxq))
+ xdp_rxq_info_unreg(&dring->xdp_rxq);
+
memset(dring->desc, 0, sizeof(struct netsec_desc) * DESC_NUM);
memset(dring->vaddr, 0, DESC_SZ * DESC_NUM);
@@ -1059,7 +1059,23 @@ static void netsec_setup_tx_dring(struct netsec_priv *priv)
static int netsec_setup_rx_dring(struct netsec_priv *priv)
{
struct netsec_desc_ring *dring = &priv->desc_ring[NETSEC_RING_RX];
- int i;
+ struct page_pool_params pp_params = { 0 };
+ int i, err;
+
+ pp_params.order = 0;
+ /* internal DMA mapping in page_pool */
+ pp_params.flags = PP_FLAG_DMA_MAP;
+ pp_params.pool_size = DESC_NUM;
+ pp_params.nid = cpu_to_node(0);
+ pp_params.dev = priv->dev;
+ pp_params.dma_dir = DMA_FROM_DEVICE;
+
+ dring->page_pool = page_pool_create(&pp_params);
+ if (IS_ERR(dring->page_pool)) {
+ err = PTR_ERR(dring->page_pool);
+ dring->page_pool = NULL;
+ goto err_out;
+ }
for (i = 0; i < DESC_NUM; i++) {
struct netsec_desc *desc = &dring->desc[i];
@@ -1067,10 +1083,10 @@ static int netsec_setup_rx_dring(struct netsec_priv *priv)
void *buf;
u16 len;
- buf = netsec_alloc_rx_data(priv, &dma_handle, &len,
- false);
+ buf = netsec_alloc_rx_data(priv, &dma_handle, &len);
+
if (!buf) {
- netsec_uninit_pkt_dring(priv, NETSEC_RING_RX);
+ err = -ENOMEM;
goto err_out;
}
desc->dma_addr = dma_handle;
@@ -1079,11 +1095,22 @@ static int netsec_setup_rx_dring(struct netsec_priv *priv)
}
netsec_rx_fill(priv, 0, DESC_NUM);
+ err = xdp_rxq_info_reg(&dring->xdp_rxq, priv->ndev, 0);
+ if (err)
+ goto err_out;
+
+ err = xdp_rxq_info_reg_mem_model(&dring->xdp_rxq, MEM_TYPE_PAGE_POOL,
+ dring->page_pool);
+ if (err) {
+ page_pool_free(dring->page_pool);
+ goto err_out;
+ }
return 0;
err_out:
- return -ENOMEM;
+ netsec_uninit_pkt_dring(priv, NETSEC_RING_RX);
+ return err;
}
static int netsec_netdev_load_ucode_region(struct netsec_priv *priv, u32 reg,
--
2.20.1
^ permalink raw reply related
* [PATCH 2/3, net-next] net: page_pool: add helper function for retrieving dma direction
From: Ilias Apalodimas @ 2019-06-28 10:39 UTC (permalink / raw)
To: netdev, jaswinder.singh
Cc: ard.biesheuvel, bjorn.topel, magnus.karlsson, brouer, daniel, ast,
makita.toshiaki, jakub.kicinski, john.fastabend, davem,
maciejromanfijalkowski, Ilias Apalodimas
In-Reply-To: <1561718355-13919-1-git-send-email-ilias.apalodimas@linaro.org>
Since the dma direction is stored in page pool params, offer an API
helper for driver that choose not to keep track of it locally
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
include/net/page_pool.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/include/net/page_pool.h b/include/net/page_pool.h
index f07c518ef8a5..ee9c871d2043 100644
--- a/include/net/page_pool.h
+++ b/include/net/page_pool.h
@@ -112,6 +112,15 @@ static inline struct page *page_pool_dev_alloc_pages(struct page_pool *pool)
return page_pool_alloc_pages(pool, gfp);
}
+/* get the stored dma direction. A driver might decide to treat this locally and
+ * avoid the extra cache line from page_pool to determine the direction
+ */
+static
+inline enum dma_data_direction page_pool_get_dma_dir(struct page_pool *pool)
+{
+ return pool->p.dma_dir;
+}
+
struct page_pool *page_pool_create(const struct page_pool_params *params);
void __page_pool_free(struct page_pool *pool);
--
2.20.1
^ permalink raw reply related
* [PATCH 3/3, net-next] net: netsec: add XDP support
From: Ilias Apalodimas @ 2019-06-28 10:39 UTC (permalink / raw)
To: netdev, jaswinder.singh
Cc: ard.biesheuvel, bjorn.topel, magnus.karlsson, brouer, daniel, ast,
makita.toshiaki, jakub.kicinski, john.fastabend, davem,
maciejromanfijalkowski, Ilias Apalodimas
In-Reply-To: <1561718355-13919-1-git-send-email-ilias.apalodimas@linaro.org>
The interface only supports 1 Tx queue so locking is introduced on
the Tx queue if XDP is enabled to make sure .ndo_start_xmit and
.ndo_xdp_xmit won't corrupt Tx ring
- Performance (SMMU off)
Benchmark XDP_SKB XDP_DRV
xdp1 291kpps 344kpps
rxdrop 282kpps 342kpps
- Performance (SMMU on)
Benchmark XDP_SKB XDP_DRV
xdp1 167kpps 324kpps
rxdrop 164kpps 323kpps
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
drivers/net/ethernet/socionext/netsec.c | 361 ++++++++++++++++++++++--
1 file changed, 334 insertions(+), 27 deletions(-)
diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
index e653b24d0534..d200d47df1b4 100644
--- a/drivers/net/ethernet/socionext/netsec.c
+++ b/drivers/net/ethernet/socionext/netsec.c
@@ -9,6 +9,9 @@
#include <linux/etherdevice.h>
#include <linux/interrupt.h>
#include <linux/io.h>
+#include <linux/netlink.h>
+#include <linux/bpf.h>
+#include <linux/bpf_trace.h>
#include <net/tcp.h>
#include <net/page_pool.h>
@@ -236,23 +239,41 @@
#define DESC_NUM 256
#define NETSEC_SKB_PAD (NET_SKB_PAD + NET_IP_ALIGN)
-#define NETSEC_RX_BUF_NON_DATA (NETSEC_SKB_PAD + \
+#define NETSEC_RXBUF_HEADROOM (max(XDP_PACKET_HEADROOM, NET_SKB_PAD) + \
+ NET_IP_ALIGN)
+#define NETSEC_RX_BUF_NON_DATA (NETSEC_RXBUF_HEADROOM + \
SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
#define DESC_SZ sizeof(struct netsec_de)
#define NETSEC_F_NETSEC_VER_MAJOR_NUM(x) ((x) & 0xffff0000)
+#define NETSEC_XDP_PASS 0
+#define NETSEC_XDP_CONSUMED BIT(0)
+#define NETSEC_XDP_TX BIT(1)
+#define NETSEC_XDP_REDIR BIT(2)
+#define NETSEC_XDP_RX_OK (NETSEC_XDP_PASS | NETSEC_XDP_TX | NETSEC_XDP_REDIR)
+
enum ring_id {
NETSEC_RING_TX = 0,
NETSEC_RING_RX
};
+enum buf_type {
+ TYPE_NETSEC_SKB = 0,
+ TYPE_NETSEC_XDP_TX,
+ TYPE_NETSEC_XDP_NDO,
+};
+
struct netsec_desc {
- struct sk_buff *skb;
+ union {
+ struct sk_buff *skb;
+ struct xdp_frame *xdpf;
+ };
dma_addr_t dma_addr;
void *addr;
u16 len;
+ u8 buf_type;
};
struct netsec_desc_ring {
@@ -260,13 +281,17 @@ struct netsec_desc_ring {
struct netsec_desc *desc;
void *vaddr;
u16 head, tail;
+ u16 xdp_xmit; /* netsec_xdp_xmit packets */
+ bool is_xdp;
struct page_pool *page_pool;
struct xdp_rxq_info xdp_rxq;
+ spinlock_t lock; /* XDP tx queue locking */
};
struct netsec_priv {
struct netsec_desc_ring desc_ring[NETSEC_RING_MAX];
struct ethtool_coalesce et_coalesce;
+ struct bpf_prog *xdp_prog;
spinlock_t reglock; /* protect reg access */
struct napi_struct napi;
phy_interface_t phy_interface;
@@ -303,6 +328,11 @@ struct netsec_rx_pkt_info {
bool err_flag;
};
+static void netsec_set_tx_de(struct netsec_priv *priv,
+ struct netsec_desc_ring *dring,
+ const struct netsec_tx_pkt_ctrl *tx_ctrl,
+ const struct netsec_desc *desc, void *buf);
+
static void netsec_write(struct netsec_priv *priv, u32 reg_addr, u32 val)
{
writel(val, priv->ioaddr + reg_addr);
@@ -609,6 +639,9 @@ static bool netsec_clean_tx_dring(struct netsec_priv *priv)
int tail = dring->tail;
int cnt = 0;
+ if (dring->is_xdp)
+ spin_lock(&dring->lock);
+
pkts = 0;
bytes = 0;
entry = dring->vaddr + DESC_SZ * tail;
@@ -622,13 +655,23 @@ static bool netsec_clean_tx_dring(struct netsec_priv *priv)
eop = (entry->attr >> NETSEC_TX_LAST) & 1;
dma_rmb();
- dma_unmap_single(priv->dev, desc->dma_addr, desc->len,
- DMA_TO_DEVICE);
- if (eop) {
- pkts++;
+ if (desc->buf_type == TYPE_NETSEC_SKB)
+ dma_unmap_single(priv->dev, desc->dma_addr, desc->len,
+ DMA_TO_DEVICE);
+ else if (desc->buf_type == TYPE_NETSEC_XDP_NDO)
+ dma_unmap_single(priv->dev, desc->dma_addr,
+ desc->len, DMA_TO_DEVICE);
+
+ if (!eop)
+ goto next;
+
+ if (desc->buf_type == TYPE_NETSEC_SKB) {
bytes += desc->skb->len;
dev_kfree_skb(desc->skb);
+ } else {
+ xdp_return_frame(desc->xdpf);
}
+next:
/* clean up so netsec_uninit_pkt_dring() won't free the skb
* again
*/
@@ -645,6 +688,8 @@ static bool netsec_clean_tx_dring(struct netsec_priv *priv)
entry = dring->vaddr + DESC_SZ * tail;
cnt++;
}
+ if (dring->is_xdp)
+ spin_unlock(&dring->lock);
if (!cnt)
return false;
@@ -688,12 +733,13 @@ static void *netsec_alloc_rx_data(struct netsec_priv *priv,
if (!page)
return NULL;
- /* page_pool API will map the whole page, skip
- * NET_SKB_PAD + NET_IP_ALIGN for the payload
+ /* We allocate the same buffer length for XDP and non-XDP cases.
+ * page_pool API will map the whole page, skip what's needed for
+ * network payloads and/or XDP
*/
- *dma_handle = page_pool_get_dma_addr(page) + NETSEC_SKB_PAD;
- /* make sure the incoming payload fits in the page with the needed
- * NET_SKB_PAD + NET_IP_ALIGN + skb_shared_info
+ *dma_handle = page_pool_get_dma_addr(page) + NETSEC_RXBUF_HEADROOM;
+ /* Make sure the incoming payload fits in the page for XDP and non-XDP
+ * cases and reserve enough space for headroom + skb_shared_info
*/
*desc_len = PAGE_SIZE - NETSEC_RX_BUF_NON_DATA;
@@ -714,21 +760,159 @@ static void netsec_rx_fill(struct netsec_priv *priv, u16 from, u16 num)
}
}
+static void netsec_xdp_ring_tx_db(struct netsec_priv *priv, u16 pkts)
+{
+ if (likely(pkts))
+ netsec_write(priv, NETSEC_REG_NRM_TX_PKTCNT, pkts);
+}
+
+static void netsec_finalize_xdp_rx(struct netsec_priv *priv, u32 xdp_res,
+ u16 pkts)
+{
+ if (xdp_res & NETSEC_XDP_REDIR)
+ xdp_do_flush_map();
+
+ if (xdp_res & NETSEC_XDP_TX)
+ netsec_xdp_ring_tx_db(priv, pkts);
+}
+
+/* The current driver only supports 1 Txq, this should run under spin_lock() */
+static u32 netsec_xdp_queue_one(struct netsec_priv *priv,
+ struct xdp_frame *xdpf, bool is_ndo)
+
+{
+ struct netsec_desc_ring *tx_ring = &priv->desc_ring[NETSEC_RING_TX];
+ struct page *page = virt_to_page(xdpf->data);
+ struct netsec_tx_pkt_ctrl tx_ctrl = {};
+ struct netsec_desc tx_desc;
+ dma_addr_t dma_handle;
+ u16 filled;
+
+ if (tx_ring->head >= tx_ring->tail)
+ filled = tx_ring->head - tx_ring->tail;
+ else
+ filled = tx_ring->head + DESC_NUM - tx_ring->tail;
+
+ if (DESC_NUM - filled <= 1)
+ return NETSEC_XDP_CONSUMED;
+
+ if (is_ndo) {
+ /* this is for ndo_xdp_xmit, the buffer needs mapping before
+ * sending
+ */
+ dma_handle = dma_map_single(priv->dev, xdpf->data, xdpf->len,
+ DMA_TO_DEVICE);
+ if (dma_mapping_error(priv->dev, dma_handle))
+ return NETSEC_XDP_CONSUMED;
+ tx_desc.buf_type = TYPE_NETSEC_XDP_NDO;
+ } else {
+ /* This is the device Rx buffer from page_pool. No need to remap
+ * just sync and send it
+ */
+ struct netsec_desc_ring *rx_ring =
+ &priv->desc_ring[NETSEC_RING_RX];
+ enum dma_data_direction dma_dir =
+ page_pool_get_dma_dir(rx_ring->page_pool);
+
+ dma_handle = page_pool_get_dma_addr(page) +
+ NETSEC_RXBUF_HEADROOM;
+ dma_sync_single_for_device(priv->dev, dma_handle, xdpf->len,
+ dma_dir);
+ tx_desc.buf_type = TYPE_NETSEC_XDP_TX;
+ }
+
+ tx_desc.dma_addr = dma_handle;
+ tx_desc.addr = xdpf->data;
+ tx_desc.len = xdpf->len;
+
+ netsec_set_tx_de(priv, tx_ring, &tx_ctrl, &tx_desc, xdpf);
+
+ return NETSEC_XDP_TX;
+}
+
+static u32 netsec_xdp_xmit_back(struct netsec_priv *priv, struct xdp_buff *xdp)
+{
+ struct netsec_desc_ring *tx_ring = &priv->desc_ring[NETSEC_RING_TX];
+ struct xdp_frame *xdpf = convert_to_xdp_frame(xdp);
+ u32 ret;
+
+ if (unlikely(!xdpf))
+ return NETSEC_XDP_CONSUMED;
+
+ spin_lock(&tx_ring->lock);
+ ret = netsec_xdp_queue_one(priv, xdpf, false);
+ spin_unlock(&tx_ring->lock);
+
+ return ret;
+}
+
+static u32 netsec_run_xdp(struct netsec_priv *priv, struct bpf_prog *prog,
+ struct xdp_buff *xdp)
+{
+ u32 ret = NETSEC_XDP_PASS;
+ int err;
+ u32 act;
+
+ act = bpf_prog_run_xdp(prog, xdp);
+
+ switch (act) {
+ case XDP_PASS:
+ ret = NETSEC_XDP_PASS;
+ break;
+ case XDP_TX:
+ ret = netsec_xdp_xmit_back(priv, xdp);
+ if (ret != NETSEC_XDP_TX)
+ xdp_return_buff(xdp);
+ break;
+ case XDP_REDIRECT:
+ err = xdp_do_redirect(priv->ndev, xdp, prog);
+ if (!err) {
+ ret = NETSEC_XDP_REDIR;
+ } else {
+ ret = NETSEC_XDP_CONSUMED;
+ xdp_return_buff(xdp);
+ }
+ break;
+ default:
+ bpf_warn_invalid_xdp_action(act);
+ /* fall through */
+ case XDP_ABORTED:
+ trace_xdp_exception(priv->ndev, prog, act);
+ /* fall through -- handle aborts by dropping packet */
+ case XDP_DROP:
+ ret = NETSEC_XDP_CONSUMED;
+ xdp_return_buff(xdp);
+ break;
+ }
+
+ return ret;
+}
+
static int netsec_process_rx(struct netsec_priv *priv, int budget)
{
struct netsec_desc_ring *dring = &priv->desc_ring[NETSEC_RING_RX];
struct net_device *ndev = priv->ndev;
struct netsec_rx_pkt_info rx_info;
- struct sk_buff *skb;
+ enum dma_data_direction dma_dir;
+ struct bpf_prog *xdp_prog;
+ struct sk_buff *skb = NULL;
+ u16 xdp_xmit = 0;
+ u32 xdp_act = 0;
int done = 0;
+ rcu_read_lock();
+ xdp_prog = READ_ONCE(priv->xdp_prog);
+ dma_dir = page_pool_get_dma_dir(dring->page_pool);
+
while (done < budget) {
u16 idx = dring->tail;
struct netsec_de *de = dring->vaddr + (DESC_SZ * idx);
struct netsec_desc *desc = &dring->desc[idx];
struct page *page = virt_to_page(desc->addr);
+ u32 xdp_result = XDP_PASS;
u16 pkt_len, desc_len;
dma_addr_t dma_handle;
+ struct xdp_buff xdp;
void *buf_addr;
if (de->attr & (1U << NETSEC_RX_PKT_OWN_FIELD)) {
@@ -770,10 +954,26 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget)
break;
dma_sync_single_for_cpu(priv->dev, desc->dma_addr, pkt_len,
- DMA_FROM_DEVICE);
+ dma_dir);
prefetch(desc->addr);
+ xdp.data_hard_start = desc->addr;
+ xdp.data = desc->addr + NETSEC_RXBUF_HEADROOM;
+ xdp_set_data_meta_invalid(&xdp);
+ xdp.data_end = xdp.data + pkt_len;
+ xdp.rxq = &dring->xdp_rxq;
+
+ if (xdp_prog) {
+ xdp_result = netsec_run_xdp(priv, xdp_prog, &xdp);
+ if (xdp_result != NETSEC_XDP_PASS) {
+ xdp_act |= xdp_result;
+ if (xdp_result == NETSEC_XDP_TX)
+ xdp_xmit++;
+ goto next;
+ }
+ }
skb = build_skb(desc->addr, desc->len + NETSEC_RX_BUF_NON_DATA);
+
if (unlikely(!skb)) {
/* If skb fails recycle_direct will either unmap and
* free the page or refill the cache depending on the
@@ -787,27 +987,32 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget)
}
page_pool_release_page(dring->page_pool, page);
- /* Update the descriptor with the new buffer we allocated */
- desc->len = desc_len;
- desc->dma_addr = dma_handle;
- desc->addr = buf_addr;
-
- skb_reserve(skb, NETSEC_SKB_PAD);
- skb_put(skb, pkt_len);
+ skb_reserve(skb, xdp.data - xdp.data_hard_start);
+ skb_put(skb, xdp.data_end - xdp.data);
skb->protocol = eth_type_trans(skb, priv->ndev);
if (priv->rx_cksum_offload_flag &&
rx_info.rx_cksum_result == NETSEC_RX_CKSUM_OK)
skb->ip_summed = CHECKSUM_UNNECESSARY;
- if (napi_gro_receive(&priv->napi, skb) != GRO_DROP) {
+next:
+ if ((skb && napi_gro_receive(&priv->napi, skb) != GRO_DROP) ||
+ xdp_result & NETSEC_XDP_RX_OK) {
ndev->stats.rx_packets++;
- ndev->stats.rx_bytes += pkt_len;
+ ndev->stats.rx_bytes += xdp.data_end - xdp.data;
}
+ /* Update the descriptor with fresh buffers */
+ desc->len = desc_len;
+ desc->dma_addr = dma_handle;
+ desc->addr = buf_addr;
+
netsec_rx_fill(priv, idx, 1);
dring->tail = (dring->tail + 1) % DESC_NUM;
}
+ netsec_finalize_xdp_rx(priv, xdp_act, xdp_xmit);
+
+ rcu_read_unlock();
return done;
}
@@ -837,8 +1042,7 @@ static int netsec_napi_poll(struct napi_struct *napi, int budget)
static void netsec_set_tx_de(struct netsec_priv *priv,
struct netsec_desc_ring *dring,
const struct netsec_tx_pkt_ctrl *tx_ctrl,
- const struct netsec_desc *desc,
- struct sk_buff *skb)
+ const struct netsec_desc *desc, void *buf)
{
int idx = dring->head;
struct netsec_de *de;
@@ -861,10 +1065,16 @@ static void netsec_set_tx_de(struct netsec_priv *priv,
de->data_buf_addr_lw = lower_32_bits(desc->dma_addr);
de->buf_len_info = (tx_ctrl->tcp_seg_len << 16) | desc->len;
de->attr = attr;
- dma_wmb();
+ /* under spin_lock if using XDP */
+ if (!dring->is_xdp)
+ dma_wmb();
dring->desc[idx] = *desc;
- dring->desc[idx].skb = skb;
+ if (desc->buf_type == TYPE_NETSEC_SKB)
+ dring->desc[idx].skb = buf;
+ else if (desc->buf_type == TYPE_NETSEC_XDP_TX ||
+ desc->buf_type == TYPE_NETSEC_XDP_NDO)
+ dring->desc[idx].xdpf = buf;
/* move head ahead */
dring->head = (dring->head + 1) % DESC_NUM;
@@ -915,8 +1125,12 @@ static netdev_tx_t netsec_netdev_start_xmit(struct sk_buff *skb,
u16 tso_seg_len = 0;
int filled;
+ if (dring->is_xdp)
+ spin_lock_bh(&dring->lock);
filled = netsec_desc_used(dring);
if (netsec_check_stop_tx(priv, filled)) {
+ if (dring->is_xdp)
+ spin_unlock_bh(&dring->lock);
net_warn_ratelimited("%s %s Tx queue full\n",
dev_name(priv->dev), ndev->name);
return NETDEV_TX_BUSY;
@@ -949,6 +1163,8 @@ static netdev_tx_t netsec_netdev_start_xmit(struct sk_buff *skb,
tx_desc.dma_addr = dma_map_single(priv->dev, skb->data,
skb_headlen(skb), DMA_TO_DEVICE);
if (dma_mapping_error(priv->dev, tx_desc.dma_addr)) {
+ if (dring->is_xdp)
+ spin_unlock_bh(&dring->lock);
netif_err(priv, drv, priv->ndev,
"%s: DMA mapping failed\n", __func__);
ndev->stats.tx_dropped++;
@@ -957,11 +1173,14 @@ static netdev_tx_t netsec_netdev_start_xmit(struct sk_buff *skb,
}
tx_desc.addr = skb->data;
tx_desc.len = skb_headlen(skb);
+ tx_desc.buf_type = TYPE_NETSEC_SKB;
skb_tx_timestamp(skb);
netdev_sent_queue(priv->ndev, skb->len);
netsec_set_tx_de(priv, dring, &tx_ctrl, &tx_desc, skb);
+ if (dring->is_xdp)
+ spin_unlock_bh(&dring->lock);
netsec_write(priv, NETSEC_REG_NRM_TX_PKTCNT, 1); /* submit another tx */
return NETDEV_TX_OK;
@@ -1042,6 +1261,7 @@ static int netsec_alloc_dring(struct netsec_priv *priv, enum ring_id id)
static void netsec_setup_tx_dring(struct netsec_priv *priv)
{
struct netsec_desc_ring *dring = &priv->desc_ring[NETSEC_RING_TX];
+ struct bpf_prog *xdp_prog = READ_ONCE(priv->xdp_prog);
int i;
for (i = 0; i < DESC_NUM; i++) {
@@ -1054,11 +1274,18 @@ static void netsec_setup_tx_dring(struct netsec_priv *priv)
*/
de->attr = 1U << NETSEC_TX_SHIFT_OWN_FIELD;
}
+
+ if (xdp_prog)
+ dring->is_xdp = true;
+ else
+ dring->is_xdp = false;
+
}
static int netsec_setup_rx_dring(struct netsec_priv *priv)
{
struct netsec_desc_ring *dring = &priv->desc_ring[NETSEC_RING_RX];
+ struct bpf_prog *xdp_prog = READ_ONCE(priv->xdp_prog);
struct page_pool_params pp_params = { 0 };
int i, err;
@@ -1068,7 +1295,7 @@ static int netsec_setup_rx_dring(struct netsec_priv *priv)
pp_params.pool_size = DESC_NUM;
pp_params.nid = cpu_to_node(0);
pp_params.dev = priv->dev;
- pp_params.dma_dir = DMA_FROM_DEVICE;
+ pp_params.dma_dir = xdp_prog ? DMA_BIDIRECTIONAL : DMA_FROM_DEVICE;
dring->page_pool = page_pool_create(&pp_params);
if (IS_ERR(dring->page_pool)) {
@@ -1490,6 +1717,9 @@ static int netsec_netdev_init(struct net_device *ndev)
if (ret)
goto err2;
+ spin_lock_init(&priv->desc_ring[NETSEC_RING_TX].lock);
+ spin_lock_init(&priv->desc_ring[NETSEC_RING_RX].lock);
+
return 0;
err2:
netsec_free_dring(priv, NETSEC_RING_RX);
@@ -1522,6 +1752,81 @@ static int netsec_netdev_ioctl(struct net_device *ndev, struct ifreq *ifr,
return phy_mii_ioctl(ndev->phydev, ifr, cmd);
}
+static int netsec_xdp_xmit(struct net_device *ndev, int n,
+ struct xdp_frame **frames, u32 flags)
+{
+ struct netsec_priv *priv = netdev_priv(ndev);
+ struct netsec_desc_ring *tx_ring = &priv->desc_ring[NETSEC_RING_TX];
+ int drops = 0;
+ int i;
+
+ if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK))
+ return -EINVAL;
+
+ spin_lock(&tx_ring->lock);
+ for (i = 0; i < n; i++) {
+ struct xdp_frame *xdpf = frames[i];
+ int err;
+
+ err = netsec_xdp_queue_one(priv, xdpf, true);
+ if (err != NETSEC_XDP_TX) {
+ xdp_return_frame_rx_napi(xdpf);
+ drops++;
+ } else {
+ tx_ring->xdp_xmit++;
+ }
+ }
+ spin_unlock(&tx_ring->lock);
+
+ if (unlikely(flags & XDP_XMIT_FLUSH)) {
+ netsec_xdp_ring_tx_db(priv, tx_ring->xdp_xmit);
+ tx_ring->xdp_xmit = 0;
+ }
+
+ return n - drops;
+}
+
+static int netsec_xdp_setup(struct netsec_priv *priv, struct bpf_prog *prog,
+ struct netlink_ext_ack *extack)
+{
+ struct net_device *dev = priv->ndev;
+ struct bpf_prog *old_prog;
+
+ /* For now just support only the usual MTU sized frames */
+ if (prog && dev->mtu > 1500) {
+ NL_SET_ERR_MSG_MOD(extack, "Jumbo frames not supported on XDP");
+ return -EOPNOTSUPP;
+ }
+
+ if (netif_running(dev))
+ netsec_netdev_stop(dev);
+
+ /* Detach old prog, if any */
+ old_prog = xchg(&priv->xdp_prog, prog);
+ if (old_prog)
+ bpf_prog_put(old_prog);
+
+ if (netif_running(dev))
+ netsec_netdev_open(dev);
+
+ return 0;
+}
+
+static int netsec_xdp(struct net_device *ndev, struct netdev_bpf *xdp)
+{
+ struct netsec_priv *priv = netdev_priv(ndev);
+
+ switch (xdp->command) {
+ case XDP_SETUP_PROG:
+ return netsec_xdp_setup(priv, xdp->prog, xdp->extack);
+ case XDP_QUERY_PROG:
+ xdp->prog_id = priv->xdp_prog ? priv->xdp_prog->aux->id : 0;
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
+
static const struct net_device_ops netsec_netdev_ops = {
.ndo_init = netsec_netdev_init,
.ndo_uninit = netsec_netdev_uninit,
@@ -1532,6 +1837,8 @@ static const struct net_device_ops netsec_netdev_ops = {
.ndo_set_mac_address = eth_mac_addr,
.ndo_validate_addr = eth_validate_addr,
.ndo_do_ioctl = netsec_netdev_ioctl,
+ .ndo_xdp_xmit = netsec_xdp_xmit,
+ .ndo_bpf = netsec_xdp,
};
static int netsec_of_probe(struct platform_device *pdev,
--
2.20.1
^ permalink raw reply related
* Re: VLAN tags in mac_len
From: Johannes Berg @ 2019-06-28 10:55 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov
Cc: netdev, bridge, nikolay, roopa, jhs, David Ahern, Zahari Doychev,
Simon Horman, Toshiaki Makita, Cong Wang, Jiri Pirko
In-Reply-To: <fa2b9787-c658-ac49-1c35-0d84d52b3ec1@iogearbox.net>
On Mon, 2019-06-17 at 10:49 +0200, Daniel Borkmann wrote:
> > > > Any other thoughts?
> > >
> > > imo skb_vlan_push() should still change mac_len.
> > > tc, ovs, bpf use it and expect vlan to be part of L2.
> >
> > I'm not sure tc really cares, but it *is* a reasonable argument to make.
> >
> > Like I said, whichever way I look at the problem, a different solution
> > looks more reasonable ;-)
>
> Agree with Alexei that the approach which would be least confusing and/or
> potentially causing regressions might be to go for 1).
Toshiaki explained already that (1) [changing the bridge code] isn't
sufficient, but Zahari is going to send patches to do (1) since that
lets use disentangle the bridge code from the rest of the discussion,
basically making it able to handle anything we throw at it.
> tc *does* care at least
> for the *BPF* case. In sch_clsact we have the ingress and egress hook where we
> can attach to and programs don't need to care where they are being attached
> since for both cases they see skb->data starting at eth header! In order to
> do this, we do a __skb_push()/__skb_pull() dance based on skb->mac_len depending
> from where we come. This also means that if a program pushed/popped a vlan tag,
> this still must be correct wrt expectations for the receive side. It is essential
> that there is consistent behavior on skb->mac_len given skbs can also be redirected
> from TX->RX or RX->TX(->RX), so that we don't pull to a wrong offset next time.
As somebody (also Toshiaki I think) explained, this is already not right
and tc mirred is broken.
So I still think we have a semantic problem here with mac_len and TX/RX,
but it's not something I feel I'm competent enough to really address.
johannes
^ permalink raw reply
* Re: [Bridge] VLAN tags in mac_len
From: Johannes Berg @ 2019-06-28 11:02 UTC (permalink / raw)
To: Toshiaki Makita, Alexei Starovoitov
Cc: Toshiaki Makita, Jiri Pirko, Nikolay Aleksandrov, netdev, roopa,
bridge, Zahari Doychev, jhs, Simon Horman, David Ahern, Cong Wang
In-Reply-To: <828a23fe-8466-ae65-7829-620f32aacead@gmail.com>
On Mon, 2019-06-17 at 20:15 +0900, Toshiaki Makita wrote:
> I'll try to explain the problem I see, which cannot be fixed by option 1...
> The bug is in tcf_vlan_act(), and mainly in skb->data, not in mac_len.
>
> Consider about vlan packets from NIC, but non-hw-accelerated, where
> vlan devices are configured to receive them.
>
> When __netif_receive_skb_core() is called, skb is like this.
>
> +-----+------+--------
> > eth | vlan | TCP/IP
>
> +-----+------+--------
> ^
> data
>
> skb->data is at the beginning of the vlan header.
Right.
> This is reasonable because we did not process the vlan tag at this point.
I think with this simple sentence you just threw a whole new semantic
issue into the mix, one that I at least hadn't considered.
However, it's not clear to me whether we should consider a tag as
processed or not when we push it.
In a sense, this means we should have two different VLAN tag push
options - considering it processed or unprocessed. Or maybe it should
always be considered unprocessed, but that's not what we do today.
> Then after vlan_do_receive() (receive the skb on a vlan device), the skb is like this.
>
> +-----+--------
> > eth | TCP/IP
>
> +-----+--------
> ^
> data
>
> Or if reorder_hdr is off (which does not remove vlan tags when receiving on vlan devices),
>
> +-----+------+--------
> > eth | vlan | TCP/IP
>
> +-----+------+--------
> ^
> data
>
> Relying on this mechanism, we are currently able to handle multiple vlan tags.
>
> For example if we have 2 tags,
>
> - On __netif_receive_skb_core() invocation
>
> +-----+------+------+--------
> > eth | vlan | vlan | TCP/IP
>
> +-----+------+------+--------
> ^
> data
>
> - After first vlan_do_receive()
>
> +-----+------+--------
> > eth | vlan | TCP/IP
>
> +-----+------+--------
> ^
> data
>
> Or if reorder_hdr is off,
>
> +-----+------+------+--------
> > eth | vlan | vlan | TCP/IP
>
> +-----+------+------+--------
> ^
> data
>
> When we process one tag, the data goes forward by one tag.
Right, that's a very good point.
> Now looking at TC vlan case...
>
> After it inserts two tags, the skb looks like:
>
> (The first tag is in vlan_tci)
> +-----+------+--------
> > eth | vlan | TCP/IP
>
> +-----+------+--------
> ^
> data
>
> The data pointer went forward before we process it.
> This is apparently wrong. I think we don't want to (or cannot?) handle cases like this
> after tcf_vlan_act(). This is why I said we should remember mac_len there.
Right, makes a lot of sense.
If you consider a tc VLAN pop, you'd argue that it should pop the next
unprocessed tag I guess, since if it was processed then it doesn't
really exist any more (semantically, you still see it if reorder_hdr is
off), right?
> So, my opinion is:
> On ingress, data pointer can be at the end of vlan header and mac_len probably should
> include vlan tag length, but only after the vlan tag is processed.
You're basically arguing for option (3), I think, making VLAN push/pop
not manipulate mac_len since they can just push/pop *unprocessed* tags,
right?
I fear this will cause all kinds of trouble in other code. Perhaps we
need to make this processed/unprocessed state more explicit.
> Bridge may need to handle mac_len that is not equal to ETH_HLEN but to me it's a
> different problem.
Yes. Like I just said to Daniel, I think we should make bridge handle
mac_len so that we can just exclude it from this whole discussion.
Regardless of the mac_len and processed/unprocessed tags, it would just
work as expected.
johannes
^ permalink raw reply
* Re: [PATCH net] sctp: fix error handling on stream scheduler initialization
From: Neil Horman @ 2019-06-28 11:06 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: netdev, Xin Long, linux-sctp, Hillf Danton
In-Reply-To: <bcbc85604e53843a731a79df620d5f92b194d085.1561675505.git.marcelo.leitner@gmail.com>
On Thu, Jun 27, 2019 at 07:48:10PM -0300, Marcelo Ricardo Leitner wrote:
> It allocates the extended area for outbound streams only on sendmsg
> calls, if they are not yet allocated. When using the priority
> stream scheduler, this initialization may imply into a subsequent
> allocation, which may fail. In this case, it was aborting the stream
> scheduler initialization but leaving the ->ext pointer (allocated) in
> there, thus in a partially initialized state. On a subsequent call to
> sendmsg, it would notice the ->ext pointer in there, and trip on
> uninitialized stuff when trying to schedule the data chunk.
>
> The fix is undo the ->ext initialization if the stream scheduler
> initialization fails and avoid the partially initialized state.
>
> Although syzkaller bisected this to commit 4ff40b86262b ("sctp: set
> chunk transport correctly when it's a new asoc"), this bug was actually
> introduced on the commit I marked below.
>
> Reported-by: syzbot+c1a380d42b190ad1e559@syzkaller.appspotmail.com
> Fixes: 5bbbbe32a431 ("sctp: introduce stream scheduler foundations")
> Tested-by: Xin Long <lucien.xin@gmail.com>
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
> net/sctp/stream.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/net/sctp/stream.c b/net/sctp/stream.c
> index 93ed07877337eace4ef7f4775dda5868359ada37..25946604af85c09917e63e5c4a8d7d6fa2caebc4 100644
> --- a/net/sctp/stream.c
> +++ b/net/sctp/stream.c
> @@ -153,13 +153,20 @@ int sctp_stream_init(struct sctp_stream *stream, __u16 outcnt, __u16 incnt,
> int sctp_stream_init_ext(struct sctp_stream *stream, __u16 sid)
> {
> struct sctp_stream_out_ext *soute;
> + int ret;
>
> soute = kzalloc(sizeof(*soute), GFP_KERNEL);
> if (!soute)
> return -ENOMEM;
> SCTP_SO(stream, sid)->ext = soute;
>
> - return sctp_sched_init_sid(stream, sid, GFP_KERNEL);
> + ret = sctp_sched_init_sid(stream, sid, GFP_KERNEL);
> + if (ret) {
> + kfree(SCTP_SO(stream, sid)->ext);
> + SCTP_SO(stream, sid)->ext = NULL;
> + }
> +
> + return ret;
> }
>
> void sctp_stream_free(struct sctp_stream *stream)
> --
> 2.21.0
>
>
Acked-by: Neil Horman <nhorman@redhat.com>
^ permalink raw reply
* Re: [RFC] longer netdev names proposal
From: Jiri Pirko @ 2019-06-28 11:12 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Michal Kubecek, netdev, Andrew Lunn, David Ahern, davem,
jakub.kicinski, mlxsw
In-Reply-To: <20190627122041.18c46daf@hermes.lan>
Thu, Jun 27, 2019 at 09:20:41PM CEST, stephen@networkplumber.org wrote:
>On Thu, 27 Jun 2019 20:39:48 +0200
>Michal Kubecek <mkubecek@suse.cz> wrote:
>
>> >
>> > $ ip li set dev enp3s0 alias "Onboard Ethernet"
>> > # ip link show "Onboard Ethernet"
>> > Device "Onboard Ethernet" does not exist.
>> >
>> > So it does not really appear to be an alias, it is a label. To be
>> > truly useful, it needs to be more than a label, it needs to be a real
>> > alias which you can use.
>>
>> That's exactly what I meant: to be really useful, one should be able to
>> use the alias(es) for setting device options, for adding routes, in
>> netfilter rules etc.
>>
>> Michal
>
>The kernel doesn't enforce uniqueness of alias.
>Also current kernel RTM_GETLINK doesn't do filter by alias (easily fixed).
>
>If it did, then handling it in iproute would be something like:
I think that it is desired for kernel to work with "real alias" as a
handle. Userspace could either pass ifindex, IFLA_NAME or "real alias".
Userspace mapping like you did here might be perhaps okay for iproute2,
but I think that we need something and easy to use for all.
Let's call it "altname". Get would return:
IFLA_NAME eth0
IFLA_ALT_NAME_LIST
IFLA_ALT_NAME eth0
IFLA_ALT_NAME somethingelse
IFLA_ALT_NAME somenamethatisreallylong
then userspace would pass with a request (get/set/del):
IFLA_ALT_NAME eth0/somethingelse/somenamethatisreallylong
or
IFLA_NAME eth0 if it is talking with older kernel
Then following would do exactly the same:
ip link set eth0 addr 11:22:33:44:55:66
ip link set somethingelse addr 11:22:33:44:55:66
ip link set somenamethatisreallylong addr 11:22:33:44:55:66
We would have to figure out the iproute2 iface to add/del altnames:
ip link add eth0 altname somethingelse
ip link del eth0 altname somethingelse
this might be also:
ip link del somethingelse altname somethingelse
How does this sound?
^ permalink raw reply
* Re: [RFC] longer netdev names proposal
From: Michal Kubecek @ 2019-06-28 11:42 UTC (permalink / raw)
To: netdev
Cc: Jiri Pirko, Stephen Hemminger, Andrew Lunn, David Ahern, davem,
jakub.kicinski, mlxsw
In-Reply-To: <20190628111216.GA2568@nanopsycho>
On Fri, Jun 28, 2019 at 01:12:16PM +0200, Jiri Pirko wrote:
>
> I think that it is desired for kernel to work with "real alias" as a
> handle. Userspace could either pass ifindex, IFLA_NAME or "real alias".
> Userspace mapping like you did here might be perhaps okay for iproute2,
> but I think that we need something and easy to use for all.
>
> Let's call it "altname". Get would return:
>
> IFLA_NAME eth0
> IFLA_ALT_NAME_LIST
> IFLA_ALT_NAME eth0
> IFLA_ALT_NAME somethingelse
> IFLA_ALT_NAME somenamethatisreallylong
>
> then userspace would pass with a request (get/set/del):
> IFLA_ALT_NAME eth0/somethingelse/somenamethatisreallylong
> or
> IFLA_NAME eth0 if it is talking with older kernel
>
> Then following would do exactly the same:
> ip link set eth0 addr 11:22:33:44:55:66
> ip link set somethingelse addr 11:22:33:44:55:66
> ip link set somenamethatisreallylong addr 11:22:33:44:55:66
Yes, this sounds nice.
> We would have to figure out the iproute2 iface to add/del altnames:
> ip link add eth0 altname somethingelse
> ip link del eth0 altname somethingelse
> this might be also:
> ip link del somethingelse altname somethingelse
This would be a bit confusing, IMHO, as so far
ip link add $name ...
always means we want to add or delete new device $name which would not
be the case here. How about the other way around:
ip link add somethingelse altname_for eth0
(preferrably with a better keyword than "altname_for" :-) ). Or maybe
ip altname add somethingelse dev eth0
ip altname del somethingelse dev eth0
Michal
^ permalink raw reply
* Re: [RFC PATCH 1/1] Documentation: net: dsa: b53: Describe b53 configuration
From: Kurt Kanzenbach @ 2019-06-28 11:44 UTC (permalink / raw)
To: Florian Fainelli
Cc: Benedikt Spranger, netdev, Sebastian Andrzej Siewior, Andrew Lunn,
Vivien Didelot
In-Reply-To: <5fe6c1b8-6273-be3d-cf75-6efdd7f9b27d@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2077 bytes --]
Hi,
On Thu, Jun 27, 2019 at 09:38:16AM -0700, Florian Fainelli wrote:
> On 6/27/19 3:15 AM, Benedikt Spranger wrote:
> > Document the different needs of documentation for the b53 driver.
> >
> > Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
> > ---
> > Documentation/networking/dsa/b53.rst | 300 +++++++++++++++++++++++++++
> > 1 file changed, 300 insertions(+)
> > create mode 100644 Documentation/networking/dsa/b53.rst
> >
> > diff --git a/Documentation/networking/dsa/b53.rst b/Documentation/networking/dsa/b53.rst
> > new file mode 100644
> > index 000000000000..5838cf6230da
> > --- /dev/null
> > +++ b/Documentation/networking/dsa/b53.rst
> > @@ -0,0 +1,300 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +==========================================
> > +Broadcom RoboSwitch Ethernet switch driver
> > +==========================================
> > +
> > +The Broadcom RoboSwitch Ethernet switch family is used in quite a range of
> > +xDSL router, cable modems and other multimedia devices.
> > +
> > +The actual implementation supports the devices BCM5325E, BCM5365, BCM539x,
> > +BCM53115 and BCM53125 as well as BCM63XX.
> > +
> > +Implementation details
> > +======================
> > +
> > +The driver is located in ``drivers/net/dsa/bcm_sf2.c`` and is implemented as a
> > +DSA driver; see ``Documentation/networking/dsa/dsa.rst`` for details on the
> > +subsystemand what it provides.
>
> The driver is under drivers/net/dsa/b53/
>
> s/ethernet/Ethernet/ for your global submission.
>
> What you are describing is not entirely specific to the B53 driver
> (maybe with the exception of having a VLAN tag on the DSA master network
> device, since B53 puts the CPU port tagged in all VLANs by default), and
> therefore the entire document should be written with the general DSA
> devices in mind, and eventually pointing out where B53 differs in a
> separate document.
That's true. It makes sense to split the documentation into a generic
and the b53 specific part.
Thanks a lot, Bene, for doing this. It's really helpful.
Thanks,
Kurt
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: XDP multi-buffer incl. jumbo-frames (Was: [RFC V1 net-next 1/1] net: ena: implement XDP drop support)
From: Eelco Chaudron @ 2019-06-28 11:49 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Jesper Dangaard Brouer, Machulsky, Zorik, Jubran, Samih, davem,
netdev, Woodhouse, David, Matushevsky, Alexander, Bshara, Saeed,
Wilson, Matt, Liguori, Anthony, Bshara, Nafea, Tzalik, Guy,
Belgazal, Netanel, Saidi, Ali, Herrenschmidt, Benjamin,
Kiyanovski, Arthur, Daniel Borkmann, Ilias Apalodimas,
Alexei Starovoitov, Jakub Kicinski, xdp-newbies
In-Reply-To: <87y31m884a.fsf@toke.dk>
On 28 Jun 2019, at 9:46, Toke Høiland-Jørgensen wrote:
> "Eelco Chaudron" <echaudro@redhat.com> writes:
>
>> On 26 Jun 2019, at 10:38, Jesper Dangaard Brouer wrote:
>>
>>> On Tue, 25 Jun 2019 03:19:22 +0000
>>> "Machulsky, Zorik" <zorik@amazon.com> wrote:
>>>
>>>> On 6/23/19, 7:21 AM, "Jesper Dangaard Brouer"
>>>> <brouer@redhat.com>
>>>> wrote:
>>>>
>>>> On Sun, 23 Jun 2019 10:06:49 +0300 <sameehj@amazon.com> wrote:
>>>>
>>>> > This commit implements the basic functionality of drop/pass
>>>> logic in the
>>>> > ena driver.
>>>>
>>>> Usually we require a driver to implement all the XDP return
>>>> codes,
>>>> before we accept it. But as Daniel and I discussed with Zorik
>>>> during
>>>> NetConf[1], we are going to make an exception and accept the
>>>> driver
>>>> if you also implement XDP_TX.
>>>>
>>>> As we trust that Zorik/Amazon will follow and implement
>>>> XDP_REDIRECT
>>>> later, given he/you wants AF_XDP support which requires
>>>> XDP_REDIRECT.
>>>>
>>>> Jesper, thanks for your comments and very helpful discussion during
>>>> NetConf! That's the plan, as we agreed. From our side I would like
>>>> to
>>>> reiterate again the importance of multi-buffer support by xdp
>>>> frame.
>>>> We would really prefer not to see our MTU shrinking because of xdp
>>>> support.
>>>
>>> Okay we really need to make a serious attempt to find a way to
>>> support
>>> multi-buffer packets with XDP. With the important criteria of not
>>> hurting performance of the single-buffer per packet design.
>>>
>>> I've created a design document[2], that I will update based on our
>>> discussions: [2]
>>> https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp-multi-buffer01-design.org
>>>
>>> The use-case that really convinced me was Eric's packet
>>> header-split.
>>>
>>>
>>> Lets refresh: Why XDP don't have multi-buffer support:
>>>
>>> XDP is designed for maximum performance, which is why certain
>>> driver-level
>>> use-cases were not supported, like multi-buffer packets (like
>>> jumbo-frames).
>>> As it e.g. complicated the driver RX-loop and memory model handling.
>>>
>>> The single buffer per packet design, is also tied into eBPF
>>> Direct-Access
>>> (DA) to packet data, which can only be allowed if the packet memory
>>> is
>>> in
>>> contiguous memory. This DA feature is essential for XDP
>>> performance.
>>>
>>>
>>> One way forward is to define that XDP only get access to the first
>>> packet buffer, and it cannot see subsequent buffers. For XDP_TX and
>>> XDP_REDIRECT to work then XDP still need to carry pointers (plus
>>> len+offset) to the other buffers, which is 16 bytes per extra
>>> buffer.
>>
>>
>> I’ve seen various network processor HW designs, and they normally
>> get
>> the first x bytes (128 - 512) which they can manipulate
>> (append/prepend/insert/modify/delete).
>>
>> There are designs where they can “page in” the additional
>> fragments
>> but it’s expensive as it requires additional memory transfers. But
>> the
>> majority do not care (cannot change) the remaining fragments. Can
>> also
>> not think of a reason why you might want to remove something at the
>> end
>> of the frame (thinking about routing/forwarding needs here).
>>
>> If we do want XDP to access other fragments we could do this through
>> a
>> helper which swaps the packet context?
>
> Yeah, I was also going to suggest a helper for that. It doesn't
> necessarily need to swap the packet context, it could just return a
> new
> pointer?
Yes that will work, my head was still thinking ASICs where there is
limited SRAM space…
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox