* [PATCH bpf-next] selftests/bpf: fix -Wstrict-aliasing in test_sockopt_sk.c
From: Stanislav Fomichev @ 2019-06-28 1:12 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev
Let's use union with u8[4] and u32 members for sockopt buffer,
that should fix any possible aliasing issues.
test_sockopt_sk.c: In function ‘getsetsockopt’:
test_sockopt_sk.c:115:2: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
if (*(__u32 *)buf != 0x55AA*2) {
^~
test_sockopt_sk.c:116:3: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
log_err("Unexpected getsockopt(SO_SNDBUF) 0x%x != 0x55AA*2",
^~~~~~~
Fixes: 8a027dc0d8f5 ("selftests/bpf: add sockopt test that exercises sk helpers")
Reported-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/test_sockopt_sk.c | 51 +++++++++----------
1 file changed, 24 insertions(+), 27 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_sockopt_sk.c b/tools/testing/selftests/bpf/test_sockopt_sk.c
index 12e79ed075ce..036b652e5ca9 100644
--- a/tools/testing/selftests/bpf/test_sockopt_sk.c
+++ b/tools/testing/selftests/bpf/test_sockopt_sk.c
@@ -22,7 +22,10 @@
static int getsetsockopt(void)
{
int fd, err;
- char buf[4] = {};
+ union {
+ char u8[4];
+ __u32 u32;
+ } buf = {};
socklen_t optlen;
fd = socket(AF_INET, SOCK_STREAM, 0);
@@ -33,31 +36,31 @@ static int getsetsockopt(void)
/* IP_TOS - BPF bypass */
- buf[0] = 0x08;
- err = setsockopt(fd, SOL_IP, IP_TOS, buf, 1);
+ buf.u8[0] = 0x08;
+ err = setsockopt(fd, SOL_IP, IP_TOS, &buf, 1);
if (err) {
log_err("Failed to call setsockopt(IP_TOS)");
goto err;
}
- buf[0] = 0x00;
+ buf.u8[0] = 0x00;
optlen = 1;
- err = getsockopt(fd, SOL_IP, IP_TOS, buf, &optlen);
+ err = getsockopt(fd, SOL_IP, IP_TOS, &buf, &optlen);
if (err) {
log_err("Failed to call getsockopt(IP_TOS)");
goto err;
}
- if (buf[0] != 0x08) {
+ if (buf.u8[0] != 0x08) {
log_err("Unexpected getsockopt(IP_TOS) buf[0] 0x%02x != 0x08",
- buf[0]);
+ buf.u8[0]);
goto err;
}
/* IP_TTL - EPERM */
- buf[0] = 1;
- err = setsockopt(fd, SOL_IP, IP_TTL, buf, 1);
+ buf.u8[0] = 1;
+ err = setsockopt(fd, SOL_IP, IP_TTL, &buf, 1);
if (!err || errno != EPERM) {
log_err("Unexpected success from setsockopt(IP_TTL)");
goto err;
@@ -65,16 +68,16 @@ static int getsetsockopt(void)
/* SOL_CUSTOM - handled by BPF */
- buf[0] = 0x01;
- err = setsockopt(fd, SOL_CUSTOM, 0, buf, 1);
+ buf.u8[0] = 0x01;
+ err = setsockopt(fd, SOL_CUSTOM, 0, &buf, 1);
if (err) {
log_err("Failed to call setsockopt");
goto err;
}
- buf[0] = 0x00;
+ buf.u32 = 0x00;
optlen = 4;
- err = getsockopt(fd, SOL_CUSTOM, 0, buf, &optlen);
+ err = getsockopt(fd, SOL_CUSTOM, 0, &buf, &optlen);
if (err) {
log_err("Failed to call getsockopt");
goto err;
@@ -84,37 +87,31 @@ static int getsetsockopt(void)
log_err("Unexpected optlen %d != 1", optlen);
goto err;
}
- if (buf[0] != 0x01) {
- log_err("Unexpected buf[0] 0x%02x != 0x01", buf[0]);
+ if (buf.u8[0] != 0x01) {
+ log_err("Unexpected buf[0] 0x%02x != 0x01", buf.u8[0]);
goto err;
}
/* SO_SNDBUF is overwritten */
- buf[0] = 0x01;
- buf[1] = 0x01;
- buf[2] = 0x01;
- buf[3] = 0x01;
- err = setsockopt(fd, SOL_SOCKET, SO_SNDBUF, buf, 4);
+ buf.u32 = 0x01010101;
+ err = setsockopt(fd, SOL_SOCKET, SO_SNDBUF, &buf, 4);
if (err) {
log_err("Failed to call setsockopt(SO_SNDBUF)");
goto err;
}
- buf[0] = 0x00;
- buf[1] = 0x00;
- buf[2] = 0x00;
- buf[3] = 0x00;
+ buf.u32 = 0x00;
optlen = 4;
- err = getsockopt(fd, SOL_SOCKET, SO_SNDBUF, buf, &optlen);
+ err = getsockopt(fd, SOL_SOCKET, SO_SNDBUF, &buf, &optlen);
if (err) {
log_err("Failed to call getsockopt(SO_SNDBUF)");
goto err;
}
- if (*(__u32 *)buf != 0x55AA*2) {
+ if (buf.u32 != 0x55AA*2) {
log_err("Unexpected getsockopt(SO_SNDBUF) 0x%x != 0x55AA*2",
- *(__u32 *)buf);
+ buf.u32);
goto err;
}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH 2/2 nf-next v3] netfilter:nft_meta: Add NFT_META_VLAN support
From: wenxu @ 2019-06-28 0:49 UTC (permalink / raw)
To: pablo, fw; +Cc: netfilter-devel, netdev
In-Reply-To: <1561682975-21790-1-git-send-email-wenxu@ucloud.cn>
From: wenxu <wenxu@ucloud.cn>
This patch provide a meta vlan to set the vlan tag of the packet.
for q-in-q outer vlan id 20:
meta vlan set 0x88a8:20
set the default 0x8100 vlan type with vlan id 20
meta vlan set 20
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
include/uapi/linux/netfilter/nf_tables.h | 4 ++++
net/netfilter/nft_meta.c | 27 ++++++++++++++++++++++++++-
2 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 0b18646..cf037f2 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -797,6 +797,7 @@ enum nft_exthdr_attributes {
* @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind)
* @NFT_META_BRI_PVID: packet input bridge port pvid
* @NFT_META_BRI_IIFVPROTO: packet input bridge vlan proto
+ * @NFT_META_VLAN: packet vlan metadata
*/
enum nft_meta_keys {
NFT_META_LEN,
@@ -829,6 +830,7 @@ enum nft_meta_keys {
NFT_META_OIFKIND,
NFT_META_BRI_PVID,
NFT_META_BRI_IIFVPROTO,
+ NFT_META_VLAN,
};
/**
@@ -895,12 +897,14 @@ enum nft_hash_attributes {
* @NFTA_META_DREG: destination register (NLA_U32)
* @NFTA_META_KEY: meta data item to load (NLA_U32: nft_meta_keys)
* @NFTA_META_SREG: source register (NLA_U32)
+ * @NFTA_META_SREG2: source register (NLA_U32)
*/
enum nft_meta_attributes {
NFTA_META_UNSPEC,
NFTA_META_DREG,
NFTA_META_KEY,
NFTA_META_SREG,
+ NFTA_META_SREG2,
__NFTA_META_MAX
};
#define NFTA_META_MAX (__NFTA_META_MAX - 1)
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index e3adf6a..29a6679 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -28,7 +28,10 @@ struct nft_meta {
enum nft_meta_keys key:8;
union {
enum nft_registers dreg:8;
- enum nft_registers sreg:8;
+ struct {
+ enum nft_registers sreg:8;
+ enum nft_registers sreg2:8;
+ };
};
};
@@ -312,6 +315,17 @@ static void nft_meta_set_eval(const struct nft_expr *expr,
skb->secmark = value;
break;
#endif
+ case NFT_META_VLAN: {
+ u32 *sreg2 = ®s->data[meta->sreg2];
+ __be16 vlan_proto;
+ u16 vlan_tci;
+
+ vlan_tci = nft_reg_load16(sreg);
+ vlan_proto = nft_reg_load16(sreg2);
+
+ __vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
+ break;
+ }
default:
WARN_ON(1);
}
@@ -321,6 +335,7 @@ static void nft_meta_set_eval(const struct nft_expr *expr,
[NFTA_META_DREG] = { .type = NLA_U32 },
[NFTA_META_KEY] = { .type = NLA_U32 },
[NFTA_META_SREG] = { .type = NLA_U32 },
+ [NFTA_META_SREG2] = { .type = NLA_U32 },
};
static int nft_meta_get_init(const struct nft_ctx *ctx,
@@ -483,6 +498,13 @@ static int nft_meta_set_init(const struct nft_ctx *ctx,
case NFT_META_PKTTYPE:
len = sizeof(u8);
break;
+ case NFT_META_VLAN:
+ len = sizeof(u16);
+ priv->sreg2 = nft_parse_register(tb[NFTA_META_SREG2]);
+ err = nft_validate_register_load(priv->sreg2, len);
+ if (err < 0)
+ return err;
+ break;
default:
return -EOPNOTSUPP;
}
@@ -521,6 +543,9 @@ static int nft_meta_set_dump(struct sk_buff *skb, const struct nft_expr *expr)
goto nla_put_failure;
if (nft_dump_register(skb, NFTA_META_SREG, priv->sreg))
goto nla_put_failure;
+ if (priv->key == NFT_META_VLAN &&
+ nft_dump_register(skb, NFTA_META_SREG2, priv->sreg2))
+ goto nla_put_failure;
return 0;
--
1.8.3.1
^ permalink raw reply related
* [PATCH 1/2 nf-next v3] netfilter: nft_meta: Add NFT_META_BRI_IIFVPROTO support
From: wenxu @ 2019-06-28 0:49 UTC (permalink / raw)
To: pablo, fw; +Cc: netfilter-devel, netdev
From: wenxu <wenxu@ucloud.cn>
This patch provide a meta to get the bridge vlan proto
nft add rule bridge firewall zones counter meta br_vlan_proto 0x8100
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
include/uapi/linux/netfilter/nf_tables.h | 2 ++
net/netfilter/nft_meta.c | 9 +++++++++
2 files changed, 11 insertions(+)
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 8859535..0b18646 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -796,6 +796,7 @@ enum nft_exthdr_attributes {
* @NFT_META_IIFKIND: packet input interface kind name (dev->rtnl_link_ops->kind)
* @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind)
* @NFT_META_BRI_PVID: packet input bridge port pvid
+ * @NFT_META_BRI_IIFVPROTO: packet input bridge vlan proto
*/
enum nft_meta_keys {
NFT_META_LEN,
@@ -827,6 +828,7 @@ enum nft_meta_keys {
NFT_META_IIFKIND,
NFT_META_OIFKIND,
NFT_META_BRI_PVID,
+ NFT_META_BRI_IIFVPROTO,
};
/**
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index 4f8116d..e3adf6a 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -248,6 +248,14 @@ void nft_meta_get_eval(const struct nft_expr *expr,
return;
}
goto err;
+ case NFT_META_BRI_IIFVPROTO:
+ if (in == NULL || (p = br_port_get_rtnl_rcu(in)) == NULL)
+ goto err;
+ if (br_opt_get(p->br, BROPT_VLAN_ENABLED)) {
+ nft_reg_store16(dest, p->br->vlan_proto);
+ return;
+ }
+ goto err;
#endif
case NFT_META_IIFKIND:
if (in == NULL || in->rtnl_link_ops == NULL)
@@ -376,6 +384,7 @@ static int nft_meta_get_init(const struct nft_ctx *ctx,
len = IFNAMSIZ;
break;
case NFT_META_BRI_PVID:
+ case NFT_META_BRI_IIFVPROTO:
if (ctx->family != NFPROTO_BRIDGE)
return -EOPNOTSUPP;
len = sizeof(u16);
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 5/5] net: dsa: microchip: Replace bit RMW with regmap
From: Andrew Lunn @ 2019-06-28 0:42 UTC (permalink / raw)
To: Marek Vasut; +Cc: netdev, Florian Fainelli, Tristram Ha, Woojung Huh
In-Reply-To: <20190627215556.23768-6-marex@denx.de>
On Thu, Jun 27, 2019 at 11:55:56PM +0200, Marek Vasut wrote:
> Regmap provides read-modify-write function to update bitfields in
> registers. Replace ad-hoc read-modify-write with regmap_update_bits()
> where applicable.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH 4/5] net: dsa: microchip: Replace ksz9477_wait_alu_sta_ready polling with regmap
From: Andrew Lunn @ 2019-06-28 0:41 UTC (permalink / raw)
To: Marek Vasut; +Cc: netdev, Florian Fainelli, Tristram Ha, Woojung Huh
In-Reply-To: <20190627215556.23768-5-marex@denx.de>
On Thu, Jun 27, 2019 at 11:55:55PM +0200, Marek Vasut wrote:
> Regmap provides polling function to poll for bits in a register. This
> function is another reimplementation of polling for bit being clear in
> a register. Replace this with regmap polling function. Moreover, inline
> the function parameters, as the function is never called with any other
> parameter values than this one.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH 3/5] net: dsa: microchip: Replace ksz9477_wait_alu_ready polling with regmap
From: Andrew Lunn @ 2019-06-28 0:40 UTC (permalink / raw)
To: Marek Vasut; +Cc: netdev, Florian Fainelli, Tristram Ha, Woojung Huh
In-Reply-To: <20190627215556.23768-4-marex@denx.de>
On Thu, Jun 27, 2019 at 11:55:54PM +0200, Marek Vasut wrote:
> Regmap provides polling function to poll for bits in a register. This
> function is another reimplementation of polling for bit being clear in
> a register. Replace this with regmap polling function. Moreover, inline
> the function parameters, as the function is never called with any other
> parameter values than this one.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH 2/5] net: dsa: microchip: Replace ksz9477_wait_vlan_ctrl_ready polling with regmap
From: Andrew Lunn @ 2019-06-28 0:39 UTC (permalink / raw)
To: Marek Vasut; +Cc: netdev, Florian Fainelli, Tristram Ha, Woojung Huh
In-Reply-To: <20190627215556.23768-3-marex@denx.de>
On Thu, Jun 27, 2019 at 11:55:53PM +0200, Marek Vasut wrote:
> Regmap provides polling function to poll for bits in a register. This
> function is another reimplementation of polling for bit being clear in
> a register. Replace this with regmap polling function. Moreover, inline
> the function parameters, as the function is never called with any other
> parameter values than this one.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH 1/5] net: dsa: microchip: Replace ad-hoc polling with regmap
From: Andrew Lunn @ 2019-06-28 0:38 UTC (permalink / raw)
To: Marek Vasut; +Cc: netdev, Florian Fainelli, Tristram Ha, Woojung Huh
In-Reply-To: <20190627215556.23768-2-marex@denx.de>
On Thu, Jun 27, 2019 at 11:55:52PM +0200, Marek Vasut wrote:
> Regmap provides polling function to poll for bits in a register,
> use in instead of reimplementing it.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH bpf-next v3 1/2] xsk: remove AF_XDP socket from map when the socket is released
From: Daniel Borkmann @ 2019-06-28 0:33 UTC (permalink / raw)
To: Björn Töpel, ast, netdev
Cc: Björn Töpel, magnus.karlsson, bruce.richardson,
songliubraving, bpf
In-Reply-To: <20190620100652.31283-2-bjorn.topel@gmail.com>
On 06/20/2019 12:06 PM, Björn Töpel wrote:
> From: Björn Töpel <bjorn.topel@intel.com>
>
> When an AF_XDP socket is released/closed the XSKMAP still holds a
> reference to the socket in a "released" state. The socket will still
> use the netdev queue resource, and block newly created sockets from
> attaching to that queue, but no user application can access the
> fill/complete/rx/tx queues. This results in that all applications need
> to explicitly clear the map entry from the old "zombie state"
> socket. This should be done automatically.
>
> After this patch, when a socket is released, it will remove itself
> from all the XSKMAPs it resides in, allowing the socket application to
> remove the code that cleans the XSKMAP entry.
>
> This behavior is also closer to that of SOCKMAP, making the two socket
> maps more consistent.
>
> Suggested-by: Bruce Richardson <bruce.richardson@intel.com>
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
Sorry for the bit of delay in reviewing, few comments inline:
> ---
> include/net/xdp_sock.h | 3 ++
> kernel/bpf/xskmap.c | 101 +++++++++++++++++++++++++++++++++++------
> net/xdp/xsk.c | 25 ++++++++++
> 3 files changed, 116 insertions(+), 13 deletions(-)
>
> diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
> index ae0f368a62bb..011a1b08d7c9 100644
> --- a/include/net/xdp_sock.h
> +++ b/include/net/xdp_sock.h
> @@ -68,6 +68,8 @@ struct xdp_sock {
> */
> spinlock_t tx_completion_lock;
> u64 rx_dropped;
> + struct list_head map_list;
> + spinlock_t map_list_lock;
> };
>
> struct xdp_buff;
> @@ -87,6 +89,7 @@ struct xdp_umem_fq_reuse *xsk_reuseq_swap(struct xdp_umem *umem,
> struct xdp_umem_fq_reuse *newq);
> void xsk_reuseq_free(struct xdp_umem_fq_reuse *rq);
> struct xdp_umem *xdp_get_umem_from_qid(struct net_device *dev, u16 queue_id);
> +void xsk_map_delete_from_node(struct xdp_sock *xs, struct list_head *node);
>
> static inline char *xdp_umem_get_data(struct xdp_umem *umem, u64 addr)
> {
> diff --git a/kernel/bpf/xskmap.c b/kernel/bpf/xskmap.c
> index ef7338cebd18..af802c89ebab 100644
> --- a/kernel/bpf/xskmap.c
> +++ b/kernel/bpf/xskmap.c
> @@ -13,8 +13,58 @@ struct xsk_map {
> struct bpf_map map;
> struct xdp_sock **xsk_map;
> struct list_head __percpu *flush_list;
> + spinlock_t lock;
> };
>
> +/* Nodes are linked in the struct xdp_sock map_list field, and used to
> + * track which maps a certain socket reside in.
> + */
> +struct xsk_map_node {
> + struct list_head node;
> + struct xsk_map *map;
> + struct xdp_sock **map_entry;
> +};
> +
> +static struct xsk_map_node *xsk_map_node_alloc(void)
> +{
> + return kzalloc(sizeof(struct xsk_map_node), GFP_ATOMIC | __GFP_NOWARN);
> +}
> +
> +static void xsk_map_node_free(struct xsk_map_node *node)
> +{
> + kfree(node);
> +}
> +
> +static void xsk_map_node_init(struct xsk_map_node *node,
> + struct xsk_map *map,
> + struct xdp_sock **map_entry)
> +{
> + node->map = map;
> + node->map_entry = map_entry;
> +}
> +
> +static void xsk_map_add_node(struct xdp_sock *xs, struct xsk_map_node *node)
> +{
> + spin_lock_bh(&xs->map_list_lock);
> + list_add_tail(&node->node, &xs->map_list);
> + spin_unlock_bh(&xs->map_list_lock);
> +}
> +
> +static void xsk_map_del_node(struct xdp_sock *xs, struct xdp_sock **map_entry)
> +{
> + struct xsk_map_node *n, *tmp;
> +
> + spin_lock_bh(&xs->map_list_lock);
> + list_for_each_entry_safe(n, tmp, &xs->map_list, node) {
> + if (map_entry == n->map_entry) {
> + list_del(&n->node);
> + xsk_map_node_free(n);
> + }
> + }
> + spin_unlock_bh(&xs->map_list_lock);
> +
> +}
> +
> static struct bpf_map *xsk_map_alloc(union bpf_attr *attr)
> {
> struct xsk_map *m;
> @@ -34,6 +84,7 @@ static struct bpf_map *xsk_map_alloc(union bpf_attr *attr)
> return ERR_PTR(-ENOMEM);
>
> bpf_map_init_from_attr(&m->map, attr);
> + spin_lock_init(&m->lock);
>
> cost = (u64)m->map.max_entries * sizeof(struct xdp_sock *);
> cost += sizeof(struct list_head) * num_possible_cpus();
> @@ -76,15 +127,16 @@ static void xsk_map_free(struct bpf_map *map)
> bpf_clear_redirect_map(map);
> synchronize_net();
>
> + spin_lock_bh(&m->lock);
> for (i = 0; i < map->max_entries; i++) {
> - struct xdp_sock *xs;
> -
> - xs = m->xsk_map[i];
> - if (!xs)
> - continue;
> + struct xdp_sock **map_entry = &m->xsk_map[i];
> + struct xdp_sock *old_xs;
>
> - sock_put((struct sock *)xs);
> + old_xs = xchg(map_entry, NULL);
> + if (old_xs)
> + xsk_map_del_node(old_xs, map_entry);
> }
> + spin_unlock_bh(&m->lock);
>
> free_percpu(m->flush_list);
> bpf_map_area_free(m->xsk_map);
> @@ -166,7 +218,8 @@ static int xsk_map_update_elem(struct bpf_map *map, void *key, void *value,
> {
> struct xsk_map *m = container_of(map, struct xsk_map, map);
> u32 i = *(u32 *)key, fd = *(u32 *)value;
> - struct xdp_sock *xs, *old_xs;
> + struct xdp_sock *xs, *old_xs, **entry;
> + struct xsk_map_node *node;
> struct socket *sock;
> int err;
>
> @@ -193,11 +246,20 @@ static int xsk_map_update_elem(struct bpf_map *map, void *key, void *value,
> return -EOPNOTSUPP;
> }
>
> - sock_hold(sock->sk);
> + node = xsk_map_node_alloc();
> + if (!node) {
> + sockfd_put(sock);
> + return -ENOMEM;
> + }
>
> - old_xs = xchg(&m->xsk_map[i], xs);
> + spin_lock_bh(&m->lock);
> + entry = &m->xsk_map[i];
> + xsk_map_node_init(node, m, entry);
> + xsk_map_add_node(xs, node);
> + old_xs = xchg(entry, xs);
> if (old_xs)
> - sock_put((struct sock *)old_xs);
> + xsk_map_del_node(old_xs, entry);
> + spin_unlock_bh(&m->lock);
>
> sockfd_put(sock);
> return 0;
> @@ -206,19 +268,32 @@ static int xsk_map_update_elem(struct bpf_map *map, void *key, void *value,
> static int xsk_map_delete_elem(struct bpf_map *map, void *key)
> {
> struct xsk_map *m = container_of(map, struct xsk_map, map);
> - struct xdp_sock *old_xs;
> + struct xdp_sock *old_xs, **map_entry;
> int k = *(u32 *)key;
>
> if (k >= map->max_entries)
> return -EINVAL;
>
> - old_xs = xchg(&m->xsk_map[k], NULL);
> + spin_lock_bh(&m->lock);
> + map_entry = &m->xsk_map[k];
> + old_xs = xchg(map_entry, NULL);
> if (old_xs)
> - sock_put((struct sock *)old_xs);
> + xsk_map_del_node(old_xs, map_entry);
> + spin_unlock_bh(&m->lock);
>
> return 0;
> }
>
> +void xsk_map_delete_from_node(struct xdp_sock *xs, struct list_head *node)
> +{
> + struct xsk_map_node *n = list_entry(node, struct xsk_map_node, node);
> +
> + spin_lock_bh(&n->map->lock);
> + *n->map_entry = NULL;
> + spin_unlock_bh(&n->map->lock);
> + xsk_map_node_free(n);
> +}
> +
> const struct bpf_map_ops xsk_map_ops = {
> .map_alloc = xsk_map_alloc,
> .map_free = xsk_map_free,
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index a14e8864e4fa..1931d98a7754 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -335,6 +335,27 @@ static int xsk_init_queue(u32 entries, struct xsk_queue **queue,
> return 0;
> }
>
> +static struct list_head *xsk_map_list_pop(struct xdp_sock *xs)
> +{
> + struct list_head *node = NULL;
> +
> + spin_lock_bh(&xs->map_list_lock);
> + if (!list_empty(&xs->map_list)) {
> + node = xs->map_list.next;
> + list_del(node);
> + }
> + spin_unlock_bh(&xs->map_list_lock);
> + return node;
> +}
> +
> +static void xsk_delete_from_maps(struct xdp_sock *xs)
> +{
> + struct list_head *node;
> +
> + while ((node = xsk_map_list_pop(xs)))
> + xsk_map_delete_from_node(xs, node);
> +}
> +
I stared at this set for a while and I think there are still two
issues in the design unless I'm missing something obvious.
1) xs teardown and parallel map update:
- CPU0 is in xsk_release(), calls xsk_delete_from_maps().
- CPU1 is in xsk_map_update_elem(), both access the same map slot.
- CPU0 does the xsk_map_list_pop() for that given slot, gets
interrupted before calling into xsk_map_delete_from_node().
- CPU1 takes m->lock in updates, *entry = xs to the new sock,
does xsk_map_del_node() to check on the xs (which CPU0 tears
down). Given this was popped off the list, it doesn't do
anything here, all good. It unlocks m->lock and succeeds.
- CPU0 now continues in xsk_map_delete_from_node(), takes
m->lock, zeroes *n->map_entry, releases m->lock, and frees
n. However, at this point *n->map_entry contains the xs that
we've just updated on CPU1. So zero'ing it will 1) remove
the wrong entry, and ii) leak it since it goes out of reach.
2) Inconsistent use of xchg() and friends:
- AF_XDP fast-path is doing READ_ONCE(m->xsk_map[key]) without
taking m->lock. This is also why you have xchg() for example
inside m->lock region since both protect different things (should
probably be commented). However, this is not consistently used.
E.g. xsk_map_delete_from_node() or xsk_map_update_elem() have
plain assignment, so compiler could in theory happily perform
store tearing and the READ_ONCE() would see garbage. This needs
to be consistently paired.
> static int xsk_release(struct socket *sock)
> {
> struct sock *sk = sock->sk;
> @@ -354,6 +375,7 @@ static int xsk_release(struct socket *sock)
> sock_prot_inuse_add(net, sk->sk_prot, -1);
> local_bh_enable();
>
> + xsk_delete_from_maps(xs);
> if (xs->dev) {
> struct net_device *dev = xs->dev;
>
> @@ -767,6 +789,9 @@ static int xsk_create(struct net *net, struct socket *sock, int protocol,
> mutex_init(&xs->mutex);
> spin_lock_init(&xs->tx_completion_lock);
>
> + INIT_LIST_HEAD(&xs->map_list);
> + spin_lock_init(&xs->map_list_lock);
> +
> mutex_lock(&net->xdp.lock);
> sk_add_node_rcu(sk, &net->xdp.list);
> mutex_unlock(&net->xdp.lock);
>
^ permalink raw reply
* Re: [GIT] Networking
From: pr-tracker-bot @ 2019-06-28 0:30 UTC (permalink / raw)
To: David Miller; +Cc: torvalds, akpm, netdev, linux-kernel
In-Reply-To: <20190626.195006.2073691861982062351.davem@davemloft.net>
The pull request you sent on Wed, 26 Jun 2019 19:50:06 -0700 (PDT):
> git://git.kernel.org/pub/scm/linux/kernel/git/davem/net refs/heads/master
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/c84afab02c311b08b5cb8ea758cc177f81c95d11
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* Re: [PATCH 1/2] tls: remove close callback sock unlock/lock and flush_sync
From: Jakub Kicinski @ 2019-06-27 23:44 UTC (permalink / raw)
To: John Fastabend; +Cc: daniel, ast, netdev, edumazet, bpf
In-Reply-To: <156165700197.32598.17496423044615153967.stgit@john-XPS-13-9370>
On Thu, 27 Jun 2019 10:36:42 -0700, John Fastabend wrote:
> The tls close() callback currently drops the sock lock, makes a
> cancel_delayed_work_sync() call, and then relocks the sock. This
> seems suspect at best. The lock_sock() is applied to stop concurrent
> operations on the socket while tearing the sock down. Further we
> will need to add support for unhash() shortly and this complicates
> matters because the lock may or may not be held then.
>
> So to fix the above situation and simplify the next patch to add
> unhash this patch creates a function tls_sk_proto_cleanup() that
> tears down the socket without calling lock_sock/release_sock. In
> order to flush the workqueue then we do the following,
>
> - Add a new bit to ctx, BIT_TX_CLOSING that is set when the
> tls resources are being removed.
> - Check this bit before scheduling any new work. This way we
> avoid queueing new work after tear down has started.
> - With the BIT_TX_CLOSING ensuring no new work is being added
> convert the cancel_delayed_work_sync to flush_delayed_work()
> - Finally call tlx_tx_records() to complete any available records
> before,
> - releasing and removing tls ctx.
>
> The above is implemented for the software case namely any of
> the following configurations from build_protos,
>
> prot[TLS_SW][TLS_BASE]
> prot[TLS_BASE][TLS_SW]
> prot[TLS_SW][TLS_SW]
>
> The implication is a follow up patch is needed to resolve the
> hardware offload case.
>
> Tested with net selftests and bpf selftests.
>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
> include/net/tls.h | 4 ++--
> net/tls/tls_main.c | 54 ++++++++++++++++++++++++++--------------------------
> net/tls/tls_sw.c | 50 ++++++++++++++++++++++++++++++++----------------
> 3 files changed, 62 insertions(+), 46 deletions(-)
>
> diff --git a/include/net/tls.h b/include/net/tls.h
> index 4a55ce6a303f..6fe1f5c96f4a 100644
> --- a/include/net/tls.h
> +++ b/include/net/tls.h
> @@ -105,9 +105,7 @@ struct tls_device {
> enum {
> TLS_BASE,
> TLS_SW,
> -#ifdef CONFIG_TLS_DEVICE
> TLS_HW,
> -#endif
> TLS_HW_RECORD,
> TLS_NUM_CONFIG,
> };
> @@ -160,6 +158,7 @@ struct tls_sw_context_tx {
> int async_capable;
>
> #define BIT_TX_SCHEDULED 0
BTW do you understand why we track this bit separately? Just to avoid
the irq operations in the workqueue code?
> +#define BIT_TX_CLOSING 1
But since we do have the above, and I think it's tested everywhere,
wouldn't setting SCHEDULED without accentually scheduling have
effectively the same result?
> unsigned long tx_bitmask;
> };
>
> @@ -327,6 +326,7 @@ void tls_sw_close(struct sock *sk, long timeout);
> void tls_sw_free_resources_tx(struct sock *sk);
> void tls_sw_free_resources_rx(struct sock *sk);
> void tls_sw_release_resources_rx(struct sock *sk);
> +void tls_sw_release_strp_rx(struct tls_context *tls_ctx);
> int tls_sw_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
> int nonblock, int flags, int *addr_len);
> bool tls_sw_stream_read(const struct sock *sk);
> diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
> index fc81ae18cc44..51cb19e24dd9 100644
> --- a/net/tls/tls_main.c
> +++ b/net/tls/tls_main.c
> @@ -261,24 +261,9 @@ static void tls_ctx_free(struct tls_context *ctx)
> kfree(ctx);
> }
>
> -static void tls_sk_proto_close(struct sock *sk, long timeout)
> +static void tls_sk_proto_cleanup(struct sock *sk,
> + struct tls_context *ctx, long timeo)
> {
> - struct tls_context *ctx = tls_get_ctx(sk);
> - long timeo = sock_sndtimeo(sk, 0);
> - void (*sk_proto_close)(struct sock *sk, long timeout);
> - bool free_ctx = false;
> -
> - lock_sock(sk);
> - sk_proto_close = ctx->sk_proto_close;
> -
> - if (ctx->tx_conf == TLS_HW_RECORD && ctx->rx_conf == TLS_HW_RECORD)
> - goto skip_tx_cleanup;
> -
> - if (ctx->tx_conf == TLS_BASE && ctx->rx_conf == TLS_BASE) {
> - free_ctx = true;
> - goto skip_tx_cleanup;
> - }
> -
> if (!tls_complete_pending_work(sk, ctx, 0, &timeo))
> tls_handle_open_record(sk, 0);
>
> @@ -299,22 +284,37 @@ static void tls_sk_proto_close(struct sock *sk, long timeout)
> #ifdef CONFIG_TLS_DEVICE
> if (ctx->rx_conf == TLS_HW)
> tls_device_offload_cleanup_rx(sk);
> -
> - if (ctx->tx_conf != TLS_HW && ctx->rx_conf != TLS_HW) {
> -#else
> - {
> #endif
> - tls_ctx_free(ctx);
> - ctx = NULL;
> +}
> +
> +static void tls_sk_proto_close(struct sock *sk, long timeout)
> +{
> + struct tls_context *ctx = tls_get_ctx(sk);
> + long timeo = sock_sndtimeo(sk, 0);
> + void (*sk_proto_close)(struct sock *sk, long timeout);
> + bool free_ctx = false;
Set but not used?
> +
> + lock_sock(sk);
> + sk_proto_close = ctx->sk_proto_close;
> +
> + if (ctx->tx_conf == TLS_HW_RECORD && ctx->rx_conf == TLS_HW_RECORD)
> + goto skip_tx_cleanup;
> +
> + if (ctx->tx_conf == TLS_BASE && ctx->rx_conf == TLS_BASE) {
> + free_ctx = true;
> + goto skip_tx_cleanup;
> }
>
> + tls_sk_proto_cleanup(sk, ctx, timeo);
> +
> skip_tx_cleanup:
> release_sock(sk);
> + if (ctx->rx_conf == TLS_SW)
> + tls_sw_release_strp_rx(ctx);
> sk_proto_close(sk, timeout);
> - /* free ctx for TLS_HW_RECORD, used by tcp_set_state
> - * for sk->sk_prot->unhash [tls_hw_unhash]
> - */
> - if (free_ctx)
> +
> + if (ctx->tx_conf != TLS_HW && ctx->rx_conf != TLS_HW &&
> + ctx->tx_conf != TLS_HW_RECORD && ctx->rx_conf != TLS_HW_RECORD)
> tls_ctx_free(ctx);
> }
>
> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index 455a782c7658..d234a6b818e6 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -473,7 +473,8 @@ static void tls_encrypt_done(struct crypto_async_request *req, int err)
> return;
>
> /* Schedule the transmission */
> - if (!test_and_set_bit(BIT_TX_SCHEDULED, &ctx->tx_bitmask))
> + if (!test_and_set_bit(BIT_TX_SCHEDULED, &ctx->tx_bitmask) &&
> + !test_bit(BIT_TX_CLOSING, &ctx->tx_bitmask))
Probably doesn't matter but seems like CLOSING test should be before
the test_and_set().
> schedule_delayed_work(&ctx->tx_work.work, 1);
> }
>
> @@ -2058,16 +2059,26 @@ void tls_sw_free_resources_tx(struct sock *sk)
> struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
> struct tls_rec *rec, *tmp;
>
> + /* Set TX CLOSING bit to stop tx_work from being scheduled
> + * while tearing down TX context. We will flush any pending
> + * work before free'ing ctx anyways. If already set then
> + * another call is already free'ing resources.
> + */
Oh, can we get multiple calls here? Is this prep for unhash?
> + if (test_and_set_bit(BIT_TX_CLOSING, &ctx->tx_bitmask))
> + return;
> +
> /* Wait for any pending async encryptions to complete */
> smp_store_mb(ctx->async_notify, true);
> if (atomic_read(&ctx->encrypt_pending))
> crypto_wait_req(-EINPROGRESS, &ctx->async_wait);
>
> - release_sock(sk);
> - cancel_delayed_work_sync(&ctx->tx_work.work);
> - lock_sock(sk);
> -
> - /* Tx whatever records we can transmit and abandon the rest */
> + /* Flush work queue and then Tx whatever records we can
> + * transmit and abandon the rest, lock_sock(sk) must be
> + * held here. We ensure no further work is enqueue by
> + * checking CLOSING bit before queueing new work and
> + * setting it above.
> + */
> + flush_delayed_work(&ctx->tx_work.work);
> tls_tx_records(sk, -1);
>
> /* Free up un-sent records in tx_list. First, free
> @@ -2111,22 +2122,22 @@ void tls_sw_release_resources_rx(struct sock *sk)
> write_lock_bh(&sk->sk_callback_lock);
> sk->sk_data_ready = ctx->saved_data_ready;
> write_unlock_bh(&sk->sk_callback_lock);
> - release_sock(sk);
> - strp_done(&ctx->strp);
> - lock_sock(sk);
> }
> }
>
> -void tls_sw_free_resources_rx(struct sock *sk)
> +void tls_sw_release_strp_rx(struct tls_context *tls_ctx)
> {
> - struct tls_context *tls_ctx = tls_get_ctx(sk);
> struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
>
> - tls_sw_release_resources_rx(sk);
> -
> + strp_done(&ctx->strp);
> kfree(ctx);
> }
>
> +void tls_sw_free_resources_rx(struct sock *sk)
> +{
> + tls_sw_release_resources_rx(sk);
> +}
I don't understand the RX side well enough, but perhaps a separate
patch would make sense here?
> /* The work handler to transmitt the encrypted records in tx_list */
> static void tx_work_handler(struct work_struct *work)
> {
> @@ -2140,9 +2151,14 @@ static void tx_work_handler(struct work_struct *work)
> if (!test_and_clear_bit(BIT_TX_SCHEDULED, &ctx->tx_bitmask))
> return;
>
> - lock_sock(sk);
> + /* If we are running from a socket close operation then the
> + * lock is already held so we do not need to hold it.
> + */
> + if (likely(!test_bit(BIT_TX_CLOSING, &ctx->tx_bitmask)))
> + lock_sock(sk);
CPU 0 (free) CPU 1 (wq)
test_bit()
lock(sk)
set_bit()
lock(sk)
flush_work()
No?
> tls_tx_records(sk, -1);
> - release_sock(sk);
> + if (likely(!test_bit(BIT_TX_CLOSING, &ctx->tx_bitmask)))
> + release_sock(sk);
> }
>
> void tls_sw_write_space(struct sock *sk, struct tls_context *ctx)
> @@ -2152,8 +2168,8 @@ void tls_sw_write_space(struct sock *sk, struct tls_context *ctx)
> /* Schedule the transmission if tx list is ready */
> if (is_tx_ready(tx_ctx) && !sk->sk_write_pending) {
> /* Schedule the transmission */
> - if (!test_and_set_bit(BIT_TX_SCHEDULED,
> - &tx_ctx->tx_bitmask))
> + if (!test_and_set_bit(BIT_TX_SCHEDULED, &tx_ctx->tx_bitmask) &&
> + !test_bit(BIT_TX_CLOSING, &tx_ctx->tx_bitmask))
> schedule_delayed_work(&tx_ctx->tx_work.work, 0);
> }
> }
>
^ permalink raw reply
* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Andy Lutomirski @ 2019-06-27 23:42 UTC (permalink / raw)
To: Andy Lutomirski, Kees Cook, Linux API
Cc: Song Liu, Network Development, bpf, Alexei Starovoitov,
Daniel Borkmann, kernel-team, lmb, Jann Horn, Greg KH
In-Reply-To: <21894f45-70d8-dfca-8c02-044f776c5e05@kernel.org>
[sigh, I finally set up lore nntp, and I goofed some addresses. Hi
Kees and linux-api.]
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
* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Andy Lutomirski @ 2019-06-27 23:40 UTC (permalink / raw)
To: Song Liu, netdev, bpf
Cc: ast, daniel, kernel-team, lmb, jannh, gregkh, linux-abi, kees
In-Reply-To: <20190627201923.2589391-2-songliubraving@fb.com>
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
* Re: [PATCH V33 24/30] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Andy Lutomirski @ 2019-06-27 23:27 UTC (permalink / raw)
To: Stephen Smalley
Cc: James Morris, Andy Lutomirski, Matthew Garrett, linux-security,
LKML, Linux API, David Howells, Alexei Starovoitov,
Matthew Garrett, Network Development, Chun-Yi Lee,
Daniel Borkmann, LSM List
In-Reply-To: <bce70c8b-9efd-6362-d536-cfbbcf70b0b7@tycho.nsa.gov>
On Thu, Jun 27, 2019 at 7:35 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
>
> On 6/26/19 8:57 PM, Andy Lutomirski wrote:
> >
> >
> >> On Jun 26, 2019, at 1:22 PM, James Morris <jmorris@namei.org> wrote:
> >>
> >> [Adding the LSM mailing list: missed this patchset initially]
> >>
> >>> On Thu, 20 Jun 2019, Andy Lutomirski wrote:
> >>>
> >>> This patch exemplifies why I don't like this approach:
> >>>
> >>>> @@ -97,6 +97,7 @@ enum lockdown_reason {
> >>>> LOCKDOWN_INTEGRITY_MAX,
> >>>> LOCKDOWN_KCORE,
> >>>> LOCKDOWN_KPROBES,
> >>>> + LOCKDOWN_BPF,
> >>>> LOCKDOWN_CONFIDENTIALITY_MAX,
> >>>
> >>>> --- a/security/lockdown/lockdown.c
> >>>> +++ b/security/lockdown/lockdown.c
> >>>> @@ -33,6 +33,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> >>>> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> >>>> [LOCKDOWN_KCORE] = "/proc/kcore access",
> >>>> [LOCKDOWN_KPROBES] = "use of kprobes",
> >>>> + [LOCKDOWN_BPF] = "use of bpf",
> >>>> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> >>>
> >>> The text here says "use of bpf", but what this patch is *really* doing
> >>> is locking down use of BPF to read kernel memory. If the details
> >>> change, then every LSM needs to get updated, and we risk breaking user
> >>> policies that are based on LSMs that offer excessively fine
> >>> granularity.
> >>
> >> Can you give an example of how the details might change?
> >>
> >>> I'd be more comfortable if the LSM only got to see "confidentiality"
> >>> or "integrity".
> >>
> >> These are not sufficient for creating a useful policy for the SELinux
> >> case.
> >>
> >>
> >
> > I may have misunderstood, but I thought that SELinux mainly needed to allow certain privileged programs to bypass the policy. Is there a real example of what SELinux wants to do that can’t be done in the simplified model?
> >
> > The think that specifically makes me uneasy about exposing all of these precise actions to LSMs is that they will get exposed to userspace in a way that forces us to treat them as stable ABIs.
>
> There are two scenarios where finer-grained distinctions make sense:
>
> - Users may need to enable specific functionality that falls under the
> umbrella of "confidentiality" or "integrity" lockdown. Finer-grained
> lockdown reasons free them from having to make an all-or-nothing choice
> between lost functionality or no lockdown at all. This can be supported
> directly by the lockdown module without any help from SELinux or other
> security modules; we just need the ability to specify these
> finer-grained lockdown levels via the boot parameters and securityfs nodes.
>
> - Different processes/programs may need to use different sets of
> functionality restricted via lockdown confidentiality or integrity
> categories. If we have to allow all-or-none for the set of
> interfaces/functionality covered by the generic confidentiality or
> integrity categories, then we'll end up having to choose between lost
> functionality or overprivileged processes, neither of which is optimal.
>
> Is it truly the case that everything under the "confidentiality"
> category poses the same level of risk to kernel confidentiality, and
> similarly for everything under the "integrity" category? If not, then
> being able to distinguish them definitely has benefit.
>
They're really quite similar in my mind. Certainly some things in the
"integrity" category give absolutely trivial control over the kernel
(e.g. modules) while others make it quite challenging (ioperm), but
the end result is very similar. And quite a few "confidentiality"
things genuinely do allow all kernel memory to be read.
I agree that finer-grained distinctions could be useful. My concern is
that it's a tradeoff, and the other end of the tradeoff is an ABI
stability issue. If someone decides down the road that some feature
that is currently "integrity" can be split into a narrow "integrity"
feature and a "confidentiality" feature then, if the user policy knows
about the individual features, there's a risk of breaking people's
systems. If we keep the fine-grained control, do we have a clear
compatibility story?
^ permalink raw reply
* Re: [PATCH V33 24/30] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Andy Lutomirski @ 2019-06-27 23:23 UTC (permalink / raw)
To: Matthew Garrett
Cc: Stephen Smalley, James Morris, Andy Lutomirski, linux-security,
LKML, Linux API, David Howells, Alexei Starovoitov,
Network Development, Chun-Yi Lee, Daniel Borkmann, LSM List
In-Reply-To: <CACdnJuuG8cR7h9v3pNcBKsxyckAzpKuBJs1GQxsz77jk5DRoQA@mail.gmail.com>
On Thu, Jun 27, 2019 at 4:16 PM Matthew Garrett <mjg59@google.com> wrote:
>
> On Thu, Jun 27, 2019 at 1:16 PM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> > That would only allow the LSM to further lock down the system above the
> > lockdown level set at boot, not grant exemptions for specific
> > functionality/interfaces required by the user or by a specific
> > process/program. You'd have to boot with lockdown=none (or your
> > lockdown=custom suggestion) in order for the LSM to allow anything
> > covered by the integrity or confidentiality levels. And then the kernel
> > would be unprotected prior to full initialization of the LSM, including
> > policy load.
> >
> > It seems like one would want to be able to boot with lockdown=integrity
> > to protect the kernel initially, then switch over to allowing the LSM to
> > selectively override it.
>
> One option would be to allow modules to be "unstacked" at runtime, but
> there's still something of a problem here - how do you ensure that
> your userland can be trusted to load a new policy before it does so?
> If you're able to assert that your early userland is trustworthy
> (perhaps because it's in an initramfs that's part of your signed boot
> payload), there's maybe an argument that most of the lockdown
> integrity guarantees are unnecessary before handoff - just using the
> lockdown LSM to protect against attacks via kernel parameters would be
> sufficient.
I think that, if you don't trust your system enough to avoid
compromising itself before policy load, then your MAC policy is more
or less dead in the water. It seems to be that it ought to be good
enough to boot with lockdown=none and then have a real policy loaded
along with the rest of the MAC policy. Or, for applications that need
to be stricter, you accept that MAC policy can't override lockdown.
^ permalink raw reply
* Re: [PATCH V33 24/30] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Matthew Garrett @ 2019-06-27 23:16 UTC (permalink / raw)
To: Stephen Smalley
Cc: James Morris, Andy Lutomirski, Andy Lutomirski, linux-security,
LKML, Linux API, David Howells, Alexei Starovoitov,
Network Development, Chun-Yi Lee, Daniel Borkmann, LSM List
In-Reply-To: <de8b15eb-ba6c-847a-7435-42742203d4a5@tycho.nsa.gov>
On Thu, Jun 27, 2019 at 1:16 PM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> That would only allow the LSM to further lock down the system above the
> lockdown level set at boot, not grant exemptions for specific
> functionality/interfaces required by the user or by a specific
> process/program. You'd have to boot with lockdown=none (or your
> lockdown=custom suggestion) in order for the LSM to allow anything
> covered by the integrity or confidentiality levels. And then the kernel
> would be unprotected prior to full initialization of the LSM, including
> policy load.
>
> It seems like one would want to be able to boot with lockdown=integrity
> to protect the kernel initially, then switch over to allowing the LSM to
> selectively override it.
One option would be to allow modules to be "unstacked" at runtime, but
there's still something of a problem here - how do you ensure that
your userland can be trusted to load a new policy before it does so?
If you're able to assert that your early userland is trustworthy
(perhaps because it's in an initramfs that's part of your signed boot
payload), there's maybe an argument that most of the lockdown
integrity guarantees are unnecessary before handoff - just using the
lockdown LSM to protect against attacks via kernel parameters would be
sufficient.
^ permalink raw reply
* [PATCH net-next 5/5] nfp: flower: add GRE encap action support
From: Jakub Kicinski @ 2019-06-27 23:12 UTC (permalink / raw)
To: davem
Cc: netdev, oss-drivers, Pieter Jansen van Vuuren, Jakub Kicinski,
John Hurley
In-Reply-To: <20190627231243.8323-1-jakub.kicinski@netronome.com>
From: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>
Add new GRE encapsulation support, which allows offload of filters
using tunnel_key set action in combination with actions that egress
to GRE type ports.
Signed-off-by: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
.../ethernet/netronome/nfp/flower/action.c | 33 ++++++++++++++++---
1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/flower/action.c b/drivers/net/ethernet/netronome/nfp/flower/action.c
index 88fedb5ada97..b6bd31fe44b2 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/action.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/action.c
@@ -170,13 +170,36 @@ nfp_fl_output(struct nfp_app *app, struct nfp_fl_output *output,
return 0;
}
+static bool
+nfp_flower_tun_is_gre(struct tc_cls_flower_offload *flow, int start_idx)
+{
+ struct flow_action_entry *act = flow->rule->action.entries;
+ int num_act = flow->rule->action.num_entries;
+ int act_idx;
+
+ /* Preparse action list for next mirred or redirect action */
+ for (act_idx = start_idx + 1; act_idx < num_act; act_idx++)
+ if (act[act_idx].id == FLOW_ACTION_REDIRECT ||
+ act[act_idx].id == FLOW_ACTION_MIRRED)
+ return netif_is_gretap(act[act_idx].dev);
+
+ return false;
+}
+
static enum nfp_flower_tun_type
nfp_fl_get_tun_from_act(struct nfp_app *app,
- const struct flow_action_entry *act)
+ struct tc_cls_flower_offload *flow,
+ const struct flow_action_entry *act, int act_idx)
{
const struct ip_tunnel_info *tun = act->tunnel;
struct nfp_flower_priv *priv = app->priv;
+ /* Determine the tunnel type based on the egress netdev
+ * in the mirred action for tunnels without l4.
+ */
+ if (nfp_flower_tun_is_gre(flow, act_idx))
+ return NFP_FL_TUNNEL_GRE;
+
switch (tun->key.tp_dst) {
case htons(IANA_VXLAN_UDP_PORT):
return NFP_FL_TUNNEL_VXLAN;
@@ -841,7 +864,7 @@ nfp_flower_loop_action(struct nfp_app *app, const struct flow_action_entry *act,
enum nfp_flower_tun_type *tun_type, int *tun_out_cnt,
int *out_cnt, u32 *csum_updated,
struct nfp_flower_pedit_acts *set_act,
- struct netlink_ext_ack *extack)
+ struct netlink_ext_ack *extack, int act_idx)
{
struct nfp_fl_set_ipv4_tun *set_tun;
struct nfp_fl_pre_tunnel *pre_tun;
@@ -896,7 +919,7 @@ nfp_flower_loop_action(struct nfp_app *app, const struct flow_action_entry *act,
case FLOW_ACTION_TUNNEL_ENCAP: {
const struct ip_tunnel_info *ip_tun = act->tunnel;
- *tun_type = nfp_fl_get_tun_from_act(app, act);
+ *tun_type = nfp_fl_get_tun_from_act(app, flow, act, act_idx);
if (*tun_type == NFP_FL_TUNNEL_NONE) {
NL_SET_ERR_MSG_MOD(extack, "unsupported offload: unsupported tunnel type in action list");
return -EOPNOTSUPP;
@@ -1022,8 +1045,8 @@ int nfp_flower_compile_action(struct nfp_app *app,
memset(&set_act, 0, sizeof(set_act));
err = nfp_flower_loop_action(app, act, flow, nfp_flow, &act_len,
netdev, &tun_type, &tun_out_cnt,
- &out_cnt, &csum_updated, &set_act,
- extack);
+ &out_cnt, &csum_updated,
+ &set_act, extack, i);
if (err)
return err;
act_cnt++;
--
2.21.0
^ permalink raw reply related
* [PATCH net-next 4/5] nfp: flower: add GRE decap classification support
From: Jakub Kicinski @ 2019-06-27 23:12 UTC (permalink / raw)
To: davem
Cc: netdev, oss-drivers, Pieter Jansen van Vuuren, Jakub Kicinski,
John Hurley
In-Reply-To: <20190627231243.8323-1-jakub.kicinski@netronome.com>
From: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>
Extend the existing tunnel matching support to include GRE decap
classification. Specifically matching existing tunnel fields for
NVGRE (GRE with protocol field set to TEB).
Signed-off-by: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
.../net/ethernet/netronome/nfp/flower/cmsg.h | 39 ++++++++++++
.../net/ethernet/netronome/nfp/flower/match.c | 44 ++++++++++++++
.../ethernet/netronome/nfp/flower/offload.c | 59 +++++++++++++------
3 files changed, 124 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/flower/cmsg.h b/drivers/net/ethernet/netronome/nfp/flower/cmsg.h
index d0d57d1ef750..0f1706ae5bfc 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/cmsg.h
+++ b/drivers/net/ethernet/netronome/nfp/flower/cmsg.h
@@ -8,6 +8,7 @@
#include <linux/skbuff.h>
#include <linux/types.h>
#include <net/geneve.h>
+#include <net/gre.h>
#include <net/vxlan.h>
#include "../nfp_app.h"
@@ -22,6 +23,7 @@
#define NFP_FLOWER_LAYER_CT BIT(6)
#define NFP_FLOWER_LAYER_VXLAN BIT(7)
+#define NFP_FLOWER_LAYER2_GRE BIT(0)
#define NFP_FLOWER_LAYER2_GENEVE BIT(5)
#define NFP_FLOWER_LAYER2_GENEVE_OP BIT(6)
@@ -37,6 +39,9 @@
#define NFP_FL_IP_FRAG_FIRST BIT(7)
#define NFP_FL_IP_FRAGMENTED BIT(6)
+/* GRE Tunnel flags */
+#define NFP_FL_GRE_FLAG_KEY BIT(2)
+
/* Compressed HW representation of TCP Flags */
#define NFP_FL_TCP_FLAG_URG BIT(4)
#define NFP_FL_TCP_FLAG_PSH BIT(3)
@@ -107,6 +112,7 @@
enum nfp_flower_tun_type {
NFP_FL_TUNNEL_NONE = 0,
+ NFP_FL_TUNNEL_GRE = 1,
NFP_FL_TUNNEL_VXLAN = 2,
NFP_FL_TUNNEL_GENEVE = 4,
};
@@ -388,6 +394,35 @@ struct nfp_flower_ipv4_udp_tun {
__be32 tun_id;
};
+/* Flow Frame GRE TUNNEL --> Tunnel details (6W/24B)
+ * -----------------------------------------------------------------
+ * 3 2 1
+ * 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | ipv4_addr_src |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | ipv4_addr_dst |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | tun_flags | tos | ttl |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | Reserved | Ethertype |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | Key |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | Reserved |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ */
+
+struct nfp_flower_ipv4_gre_tun {
+ struct nfp_flower_tun_ipv4 ipv4;
+ __be16 tun_flags;
+ struct nfp_flower_tun_ip_ext ip_ext;
+ __be16 reserved1;
+ __be16 ethertype;
+ __be32 tun_key;
+ __be32 reserved2;
+};
+
struct nfp_flower_geneve_options {
u8 data[NFP_FL_MAX_GENEVE_OPT_KEY];
};
@@ -538,6 +573,8 @@ nfp_fl_netdev_is_tunnel_type(struct net_device *netdev,
{
if (netif_is_vxlan(netdev))
return tun_type == NFP_FL_TUNNEL_VXLAN;
+ if (netif_is_gretap(netdev))
+ return tun_type == NFP_FL_TUNNEL_GRE;
if (netif_is_geneve(netdev))
return tun_type == NFP_FL_TUNNEL_GENEVE;
@@ -554,6 +591,8 @@ static inline bool nfp_fl_is_netdev_to_offload(struct net_device *netdev)
return true;
if (netif_is_geneve(netdev))
return true;
+ if (netif_is_gretap(netdev))
+ return true;
return false;
}
diff --git a/drivers/net/ethernet/netronome/nfp/flower/match.c b/drivers/net/ethernet/netronome/nfp/flower/match.c
index 9181611087c2..c1690de19172 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/match.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/match.c
@@ -316,6 +316,35 @@ nfp_flower_compile_tun_ip_ext(struct nfp_flower_tun_ip_ext *ext,
}
}
+static void
+nfp_flower_compile_ipv4_gre_tun(struct nfp_flower_ipv4_gre_tun *ext,
+ struct nfp_flower_ipv4_gre_tun *msk,
+ struct tc_cls_flower_offload *flow)
+{
+ struct flow_rule *rule = tc_cls_flower_offload_flow_rule(flow);
+
+ memset(ext, 0, sizeof(struct nfp_flower_ipv4_gre_tun));
+ memset(msk, 0, sizeof(struct nfp_flower_ipv4_gre_tun));
+
+ /* NVGRE is the only supported GRE tunnel type */
+ ext->ethertype = cpu_to_be16(ETH_P_TEB);
+ msk->ethertype = cpu_to_be16(~0);
+
+ if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_ENC_KEYID)) {
+ struct flow_match_enc_keyid match;
+
+ flow_rule_match_enc_keyid(rule, &match);
+ ext->tun_key = match.key->keyid;
+ msk->tun_key = match.mask->keyid;
+
+ ext->tun_flags = cpu_to_be16(NFP_FL_GRE_FLAG_KEY);
+ msk->tun_flags = cpu_to_be16(NFP_FL_GRE_FLAG_KEY);
+ }
+
+ nfp_flower_compile_tun_ipv4_addrs(&ext->ipv4, &msk->ipv4, flow);
+ nfp_flower_compile_tun_ip_ext(&ext->ip_ext, &msk->ip_ext, flow);
+}
+
static void
nfp_flower_compile_ipv4_udp_tun(struct nfp_flower_ipv4_udp_tun *ext,
struct nfp_flower_ipv4_udp_tun *msk,
@@ -425,6 +454,21 @@ int nfp_flower_compile_flow_match(struct nfp_app *app,
msk += sizeof(struct nfp_flower_ipv6);
}
+ if (key_ls->key_layer_two & NFP_FLOWER_LAYER2_GRE) {
+ __be32 tun_dst;
+
+ nfp_flower_compile_ipv4_gre_tun((void *)ext, (void *)msk, flow);
+ tun_dst = ((struct nfp_flower_ipv4_gre_tun *)ext)->ipv4.dst;
+ ext += sizeof(struct nfp_flower_ipv4_gre_tun);
+ msk += sizeof(struct nfp_flower_ipv4_gre_tun);
+
+ /* Store the tunnel destination in the rule data.
+ * This must be present and be an exact match.
+ */
+ nfp_flow->nfp_tun_ipv4_addr = tun_dst;
+ nfp_tunnel_add_ipv4_off(app, tun_dst);
+ }
+
if (key_ls->key_layer & NFP_FLOWER_LAYER_VXLAN ||
key_ls->key_layer_two & NFP_FLOWER_LAYER2_GENEVE) {
__be32 tun_dst;
diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c
index 6b28910442db..6dbe947269c3 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
@@ -52,8 +52,7 @@
#define NFP_FLOWER_WHITELIST_TUN_DISSECTOR_R \
(BIT(FLOW_DISSECTOR_KEY_ENC_CONTROL) | \
- BIT(FLOW_DISSECTOR_KEY_ENC_IPV4_ADDRS) | \
- BIT(FLOW_DISSECTOR_KEY_ENC_PORTS))
+ BIT(FLOW_DISSECTOR_KEY_ENC_IPV4_ADDRS))
#define NFP_FLOWER_MERGE_FIELDS \
(NFP_FLOWER_LAYER_PORT | \
@@ -285,27 +284,51 @@ nfp_flower_calculate_key_layers(struct nfp_app *app,
return -EOPNOTSUPP;
}
- flow_rule_match_enc_ports(rule, &enc_ports);
- if (enc_ports.mask->dst != cpu_to_be16(~0)) {
- NL_SET_ERR_MSG_MOD(extack, "unsupported offload: only an exact match L4 destination port is supported");
- return -EOPNOTSUPP;
- }
-
if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_ENC_OPTS))
flow_rule_match_enc_opts(rule, &enc_op);
- err = nfp_flower_calc_udp_tun_layer(enc_ports.key, enc_op.key,
- &key_layer_two, &key_layer,
- &key_size, priv, tun_type,
- extack);
- if (err)
- return err;
+ if (!flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_ENC_PORTS)) {
+ /* check if GRE, which has no enc_ports */
+ if (netif_is_gretap(netdev)) {
+ *tun_type = NFP_FL_TUNNEL_GRE;
+ key_layer |= NFP_FLOWER_LAYER_EXT_META;
+ key_size += sizeof(struct nfp_flower_ext_meta);
+ key_layer_two |= NFP_FLOWER_LAYER2_GRE;
+ key_size +=
+ sizeof(struct nfp_flower_ipv4_gre_tun);
+
+ if (enc_op.key) {
+ NL_SET_ERR_MSG_MOD(extack, "unsupported offload: encap options not supported on GRE tunnels");
+ return -EOPNOTSUPP;
+ }
+ } else {
+ NL_SET_ERR_MSG_MOD(extack, "unsupported offload: an exact match on L4 destination port is required for non-GRE tunnels");
+ return -EOPNOTSUPP;
+ }
+ } else {
+ flow_rule_match_enc_ports(rule, &enc_ports);
+ if (enc_ports.mask->dst != cpu_to_be16(~0)) {
+ NL_SET_ERR_MSG_MOD(extack, "unsupported offload: only an exact match L4 destination port is supported");
+ return -EOPNOTSUPP;
+ }
- /* Ensure the ingress netdev matches the expected tun type. */
- if (!nfp_fl_netdev_is_tunnel_type(netdev, *tun_type)) {
- NL_SET_ERR_MSG_MOD(extack, "unsupported offload: ingress netdev does not match the expected tunnel type");
- return -EOPNOTSUPP;
+ err = nfp_flower_calc_udp_tun_layer(enc_ports.key,
+ enc_op.key,
+ &key_layer_two,
+ &key_layer,
+ &key_size, priv,
+ tun_type, extack);
+ if (err)
+ return err;
+
+ /* Ensure the ingress netdev matches the expected
+ * tun type.
+ */
+ if (!nfp_fl_netdev_is_tunnel_type(netdev, *tun_type)) {
+ NL_SET_ERR_MSG_MOD(extack, "unsupported offload: ingress netdev does not match the expected tunnel type");
+ return -EOPNOTSUPP;
+ }
}
}
--
2.21.0
^ permalink raw reply related
* [PATCH net-next 3/5] nfp: flower: rename tunnel related functions in action offload
From: Jakub Kicinski @ 2019-06-27 23:12 UTC (permalink / raw)
To: davem
Cc: netdev, oss-drivers, Pieter Jansen van Vuuren, Jakub Kicinski,
John Hurley
In-Reply-To: <20190627231243.8323-1-jakub.kicinski@netronome.com>
From: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>
Previously tunnel related functions in action offload only applied
to UDP tunnels. Rename these functions in preparation for new
tunnel types.
Signed-off-by: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
.../ethernet/netronome/nfp/flower/action.c | 30 +++++++++----------
.../net/ethernet/netronome/nfp/flower/cmsg.h | 2 +-
2 files changed, 15 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/flower/action.c b/drivers/net/ethernet/netronome/nfp/flower/action.c
index 8bea3004d66c..88fedb5ada97 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/action.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/action.c
@@ -171,8 +171,8 @@ nfp_fl_output(struct nfp_app *app, struct nfp_fl_output *output,
}
static enum nfp_flower_tun_type
-nfp_fl_get_tun_from_act_l4_port(struct nfp_app *app,
- const struct flow_action_entry *act)
+nfp_fl_get_tun_from_act(struct nfp_app *app,
+ const struct flow_action_entry *act)
{
const struct ip_tunnel_info *tun = act->tunnel;
struct nfp_flower_priv *priv = app->priv;
@@ -281,15 +281,13 @@ nfp_fl_push_geneve_options(struct nfp_fl_payload *nfp_fl, int *list_len,
}
static int
-nfp_fl_set_ipv4_udp_tun(struct nfp_app *app,
- struct nfp_fl_set_ipv4_udp_tun *set_tun,
- const struct flow_action_entry *act,
- struct nfp_fl_pre_tunnel *pre_tun,
- enum nfp_flower_tun_type tun_type,
- struct net_device *netdev,
- struct netlink_ext_ack *extack)
+nfp_fl_set_ipv4_tun(struct nfp_app *app, struct nfp_fl_set_ipv4_tun *set_tun,
+ const struct flow_action_entry *act,
+ struct nfp_fl_pre_tunnel *pre_tun,
+ enum nfp_flower_tun_type tun_type,
+ struct net_device *netdev, struct netlink_ext_ack *extack)
{
- size_t act_size = sizeof(struct nfp_fl_set_ipv4_udp_tun);
+ size_t act_size = sizeof(struct nfp_fl_set_ipv4_tun);
const struct ip_tunnel_info *ip_tun = act->tunnel;
struct nfp_flower_priv *priv = app->priv;
u32 tmp_set_ip_tun_type_index = 0;
@@ -845,7 +843,7 @@ nfp_flower_loop_action(struct nfp_app *app, const struct flow_action_entry *act,
struct nfp_flower_pedit_acts *set_act,
struct netlink_ext_ack *extack)
{
- struct nfp_fl_set_ipv4_udp_tun *set_tun;
+ struct nfp_fl_set_ipv4_tun *set_tun;
struct nfp_fl_pre_tunnel *pre_tun;
struct nfp_fl_push_vlan *psh_v;
struct nfp_fl_pop_vlan *pop_v;
@@ -898,7 +896,7 @@ nfp_flower_loop_action(struct nfp_app *app, const struct flow_action_entry *act,
case FLOW_ACTION_TUNNEL_ENCAP: {
const struct ip_tunnel_info *ip_tun = act->tunnel;
- *tun_type = nfp_fl_get_tun_from_act_l4_port(app, act);
+ *tun_type = nfp_fl_get_tun_from_act(app, act);
if (*tun_type == NFP_FL_TUNNEL_NONE) {
NL_SET_ERR_MSG_MOD(extack, "unsupported offload: unsupported tunnel type in action list");
return -EOPNOTSUPP;
@@ -914,7 +912,7 @@ nfp_flower_loop_action(struct nfp_app *app, const struct flow_action_entry *act,
* If none, the packet falls back before applying other actions.
*/
if (*a_len + sizeof(struct nfp_fl_pre_tunnel) +
- sizeof(struct nfp_fl_set_ipv4_udp_tun) > NFP_FL_MAX_A_SIZ) {
+ sizeof(struct nfp_fl_set_ipv4_tun) > NFP_FL_MAX_A_SIZ) {
NL_SET_ERR_MSG_MOD(extack, "unsupported offload: maximum allowed action list size exceeded at tunnel encap");
return -EOPNOTSUPP;
}
@@ -928,11 +926,11 @@ nfp_flower_loop_action(struct nfp_app *app, const struct flow_action_entry *act,
return err;
set_tun = (void *)&nfp_fl->action_data[*a_len];
- err = nfp_fl_set_ipv4_udp_tun(app, set_tun, act, pre_tun,
- *tun_type, netdev, extack);
+ err = nfp_fl_set_ipv4_tun(app, set_tun, act, pre_tun,
+ *tun_type, netdev, extack);
if (err)
return err;
- *a_len += sizeof(struct nfp_fl_set_ipv4_udp_tun);
+ *a_len += sizeof(struct nfp_fl_set_ipv4_tun);
}
break;
case FLOW_ACTION_TUNNEL_DECAP:
diff --git a/drivers/net/ethernet/netronome/nfp/flower/cmsg.h b/drivers/net/ethernet/netronome/nfp/flower/cmsg.h
index 0d3d1b68232c..d0d57d1ef750 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/cmsg.h
+++ b/drivers/net/ethernet/netronome/nfp/flower/cmsg.h
@@ -203,7 +203,7 @@ struct nfp_fl_pre_tunnel {
__be32 extra[3];
};
-struct nfp_fl_set_ipv4_udp_tun {
+struct nfp_fl_set_ipv4_tun {
struct nfp_fl_act_head head;
__be16 reserved;
__be64 tun_id __packed;
--
2.21.0
^ permalink raw reply related
* [PATCH net-next 2/5] nfp: flower: add helper functions for tunnel classification
From: Jakub Kicinski @ 2019-06-27 23:12 UTC (permalink / raw)
To: davem
Cc: netdev, oss-drivers, Pieter Jansen van Vuuren, Jakub Kicinski,
John Hurley
In-Reply-To: <20190627231243.8323-1-jakub.kicinski@netronome.com>
From: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>
Adds IPv4 address and TTL/TOS helper functions, which is done in
preparation for compiling new tunnel types.
Signed-off-by: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
.../net/ethernet/netronome/nfp/flower/cmsg.h | 16 +++--
.../net/ethernet/netronome/nfp/flower/match.c | 59 ++++++++++++-------
2 files changed, 51 insertions(+), 24 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/flower/cmsg.h b/drivers/net/ethernet/netronome/nfp/flower/cmsg.h
index 537f7fc19584..0d3d1b68232c 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/cmsg.h
+++ b/drivers/net/ethernet/netronome/nfp/flower/cmsg.h
@@ -354,6 +354,16 @@ struct nfp_flower_ipv6 {
struct in6_addr ipv6_dst;
};
+struct nfp_flower_tun_ipv4 {
+ __be32 src;
+ __be32 dst;
+};
+
+struct nfp_flower_tun_ip_ext {
+ u8 tos;
+ u8 ttl;
+};
+
/* Flow Frame IPv4 UDP TUNNEL --> Tunnel details (4W/16B)
* -----------------------------------------------------------------
* 3 2 1
@@ -371,11 +381,9 @@ struct nfp_flower_ipv6 {
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
*/
struct nfp_flower_ipv4_udp_tun {
- __be32 ip_src;
- __be32 ip_dst;
+ struct nfp_flower_tun_ipv4 ipv4;
__be16 reserved1;
- u8 tos;
- u8 ttl;
+ struct nfp_flower_tun_ip_ext ip_ext;
__be32 reserved2;
__be32 tun_id;
};
diff --git a/drivers/net/ethernet/netronome/nfp/flower/match.c b/drivers/net/ethernet/netronome/nfp/flower/match.c
index 371b5be33dc7..9181611087c2 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/match.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/match.c
@@ -280,6 +280,42 @@ nfp_flower_compile_geneve_opt(void *ext, void *msk,
return 0;
}
+static void
+nfp_flower_compile_tun_ipv4_addrs(struct nfp_flower_tun_ipv4 *ext,
+ struct nfp_flower_tun_ipv4 *msk,
+ struct tc_cls_flower_offload *flow)
+{
+ struct flow_rule *rule = tc_cls_flower_offload_flow_rule(flow);
+
+ if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_ENC_IPV4_ADDRS)) {
+ struct flow_match_ipv4_addrs match;
+
+ flow_rule_match_enc_ipv4_addrs(rule, &match);
+ ext->src = match.key->src;
+ ext->dst = match.key->dst;
+ msk->src = match.mask->src;
+ msk->dst = match.mask->dst;
+ }
+}
+
+static void
+nfp_flower_compile_tun_ip_ext(struct nfp_flower_tun_ip_ext *ext,
+ struct nfp_flower_tun_ip_ext *msk,
+ struct tc_cls_flower_offload *flow)
+{
+ struct flow_rule *rule = tc_cls_flower_offload_flow_rule(flow);
+
+ if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_ENC_IP)) {
+ struct flow_match_ip match;
+
+ flow_rule_match_enc_ip(rule, &match);
+ ext->tos = match.key->tos;
+ ext->ttl = match.key->ttl;
+ msk->tos = match.mask->tos;
+ msk->ttl = match.mask->ttl;
+ }
+}
+
static void
nfp_flower_compile_ipv4_udp_tun(struct nfp_flower_ipv4_udp_tun *ext,
struct nfp_flower_ipv4_udp_tun *msk,
@@ -301,25 +337,8 @@ nfp_flower_compile_ipv4_udp_tun(struct nfp_flower_ipv4_udp_tun *ext,
msk->tun_id = cpu_to_be32(temp_vni);
}
- if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_ENC_IPV4_ADDRS)) {
- struct flow_match_ipv4_addrs match;
-
- flow_rule_match_enc_ipv4_addrs(rule, &match);
- ext->ip_src = match.key->src;
- ext->ip_dst = match.key->dst;
- msk->ip_src = match.mask->src;
- msk->ip_dst = match.mask->dst;
- }
-
- if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_ENC_IP)) {
- struct flow_match_ip match;
-
- flow_rule_match_enc_ip(rule, &match);
- ext->tos = match.key->tos;
- ext->ttl = match.key->ttl;
- msk->tos = match.mask->tos;
- msk->ttl = match.mask->ttl;
- }
+ nfp_flower_compile_tun_ipv4_addrs(&ext->ipv4, &msk->ipv4, flow);
+ nfp_flower_compile_tun_ip_ext(&ext->ip_ext, &msk->ip_ext, flow);
}
int nfp_flower_compile_flow_match(struct nfp_app *app,
@@ -411,7 +430,7 @@ int nfp_flower_compile_flow_match(struct nfp_app *app,
__be32 tun_dst;
nfp_flower_compile_ipv4_udp_tun((void *)ext, (void *)msk, flow);
- tun_dst = ((struct nfp_flower_ipv4_udp_tun *)ext)->ip_dst;
+ tun_dst = ((struct nfp_flower_ipv4_udp_tun *)ext)->ipv4.dst;
ext += sizeof(struct nfp_flower_ipv4_udp_tun);
msk += sizeof(struct nfp_flower_ipv4_udp_tun);
--
2.21.0
^ permalink raw reply related
* [PATCH net-next 1/5] nfp: flower: refactor tunnel key layer calculation
From: Jakub Kicinski @ 2019-06-27 23:12 UTC (permalink / raw)
To: davem
Cc: netdev, oss-drivers, Pieter Jansen van Vuuren, Jakub Kicinski,
John Hurley
In-Reply-To: <20190627231243.8323-1-jakub.kicinski@netronome.com>
From: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>
Refactor the key layer calculation function, in particular the tunnel
key layer calculation by introducing helper functions. This is done
in preparation for supporting GRE tunnel offloads.
Signed-off-by: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
.../ethernet/netronome/nfp/flower/offload.c | 100 +++++++++++-------
1 file changed, 60 insertions(+), 40 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c
index 39e6599f2bd7..6b28910442db 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
@@ -141,16 +141,16 @@ static bool nfp_flower_check_higher_than_l3(struct tc_cls_flower_offload *f)
}
static int
-nfp_flower_calc_opt_layer(struct flow_match_enc_opts *enc_opts,
+nfp_flower_calc_opt_layer(struct flow_dissector_key_enc_opts *enc_opts,
u32 *key_layer_two, int *key_size,
struct netlink_ext_ack *extack)
{
- if (enc_opts->key->len > NFP_FL_MAX_GENEVE_OPT_KEY) {
+ if (enc_opts->len > NFP_FL_MAX_GENEVE_OPT_KEY) {
NL_SET_ERR_MSG_MOD(extack, "unsupported offload: geneve options exceed maximum length");
return -EOPNOTSUPP;
}
- if (enc_opts->key->len > 0) {
+ if (enc_opts->len > 0) {
*key_layer_two |= NFP_FLOWER_LAYER2_GENEVE_OP;
*key_size += sizeof(struct nfp_flower_geneve_options);
}
@@ -158,6 +158,57 @@ nfp_flower_calc_opt_layer(struct flow_match_enc_opts *enc_opts,
return 0;
}
+static int
+nfp_flower_calc_udp_tun_layer(struct flow_dissector_key_ports *enc_ports,
+ struct flow_dissector_key_enc_opts *enc_op,
+ u32 *key_layer_two, u8 *key_layer, int *key_size,
+ struct nfp_flower_priv *priv,
+ enum nfp_flower_tun_type *tun_type,
+ struct netlink_ext_ack *extack)
+{
+ int err;
+
+ switch (enc_ports->dst) {
+ case htons(IANA_VXLAN_UDP_PORT):
+ *tun_type = NFP_FL_TUNNEL_VXLAN;
+ *key_layer |= NFP_FLOWER_LAYER_VXLAN;
+ *key_size += sizeof(struct nfp_flower_ipv4_udp_tun);
+
+ if (enc_op) {
+ NL_SET_ERR_MSG_MOD(extack, "unsupported offload: encap options not supported on vxlan tunnels");
+ return -EOPNOTSUPP;
+ }
+ break;
+ case htons(GENEVE_UDP_PORT):
+ if (!(priv->flower_ext_feats & NFP_FL_FEATS_GENEVE)) {
+ NL_SET_ERR_MSG_MOD(extack, "unsupported offload: loaded firmware does not support geneve offload");
+ return -EOPNOTSUPP;
+ }
+ *tun_type = NFP_FL_TUNNEL_GENEVE;
+ *key_layer |= NFP_FLOWER_LAYER_EXT_META;
+ *key_size += sizeof(struct nfp_flower_ext_meta);
+ *key_layer_two |= NFP_FLOWER_LAYER2_GENEVE;
+ *key_size += sizeof(struct nfp_flower_ipv4_udp_tun);
+
+ if (!enc_op)
+ break;
+ if (!(priv->flower_ext_feats & NFP_FL_FEATS_GENEVE_OPT)) {
+ NL_SET_ERR_MSG_MOD(extack, "unsupported offload: loaded firmware does not support geneve option offload");
+ return -EOPNOTSUPP;
+ }
+ err = nfp_flower_calc_opt_layer(enc_op, key_layer_two,
+ key_size, extack);
+ if (err)
+ return err;
+ break;
+ default:
+ NL_SET_ERR_MSG_MOD(extack, "unsupported offload: tunnel type unknown");
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
static int
nfp_flower_calculate_key_layers(struct nfp_app *app,
struct net_device *netdev,
@@ -243,44 +294,13 @@ nfp_flower_calculate_key_layers(struct nfp_app *app,
if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_ENC_OPTS))
flow_rule_match_enc_opts(rule, &enc_op);
- switch (enc_ports.key->dst) {
- case htons(IANA_VXLAN_UDP_PORT):
- *tun_type = NFP_FL_TUNNEL_VXLAN;
- key_layer |= NFP_FLOWER_LAYER_VXLAN;
- key_size += sizeof(struct nfp_flower_ipv4_udp_tun);
-
- if (enc_op.key) {
- NL_SET_ERR_MSG_MOD(extack, "unsupported offload: encap options not supported on vxlan tunnels");
- return -EOPNOTSUPP;
- }
- break;
- case htons(GENEVE_UDP_PORT):
- if (!(priv->flower_ext_feats & NFP_FL_FEATS_GENEVE)) {
- NL_SET_ERR_MSG_MOD(extack, "unsupported offload: loaded firmware does not support geneve offload");
- return -EOPNOTSUPP;
- }
- *tun_type = NFP_FL_TUNNEL_GENEVE;
- key_layer |= NFP_FLOWER_LAYER_EXT_META;
- key_size += sizeof(struct nfp_flower_ext_meta);
- key_layer_two |= NFP_FLOWER_LAYER2_GENEVE;
- key_size += sizeof(struct nfp_flower_ipv4_udp_tun);
- if (!enc_op.key)
- break;
- if (!(priv->flower_ext_feats &
- NFP_FL_FEATS_GENEVE_OPT)) {
- NL_SET_ERR_MSG_MOD(extack, "unsupported offload: loaded firmware does not support geneve option offload");
- return -EOPNOTSUPP;
- }
- err = nfp_flower_calc_opt_layer(&enc_op, &key_layer_two,
- &key_size, extack);
- if (err)
- return err;
- break;
- default:
- NL_SET_ERR_MSG_MOD(extack, "unsupported offload: tunnel type unknown");
- return -EOPNOTSUPP;
- }
+ err = nfp_flower_calc_udp_tun_layer(enc_ports.key, enc_op.key,
+ &key_layer_two, &key_layer,
+ &key_size, priv, tun_type,
+ extack);
+ if (err)
+ return err;
/* Ensure the ingress netdev matches the expected tun type. */
if (!nfp_fl_netdev_is_tunnel_type(netdev, *tun_type)) {
--
2.21.0
^ permalink raw reply related
* [PATCH net-next 0/5] nfp: extend flower capabilities for GRE tunnel offload
From: Jakub Kicinski @ 2019-06-27 23:12 UTC (permalink / raw)
To: davem; +Cc: netdev, oss-drivers, Jakub Kicinski
Pieter says:
This set extends the flower match and action components to offload
GRE decapsulation with classification and encapsulation actions. The
first 3 patches are refactor and cleanup patches for improving
readability and reusability. Patch 4 and 5 implement GRE decap and
encap functionality respectively.
Pieter Jansen van Vuuren (5):
nfp: flower: refactor tunnel key layer calculation
nfp: flower: add helper functions for tunnel classification
nfp: flower: rename tunnel related functions in action offload
nfp: flower: add GRE decap classification support
nfp: flower: add GRE encap action support
.../ethernet/netronome/nfp/flower/action.c | 59 +++++---
.../net/ethernet/netronome/nfp/flower/cmsg.h | 57 +++++++-
.../net/ethernet/netronome/nfp/flower/match.c | 103 +++++++++++---
.../ethernet/netronome/nfp/flower/offload.c | 133 ++++++++++++------
4 files changed, 263 insertions(+), 89 deletions(-)
--
2.21.0
^ permalink raw reply
* RE: [RFC net-next 1/5] net: stmmac: introduce IEEE 802.1Qbv configuration functionalities
From: Ong, Boon Leong @ 2019-06-27 23:08 UTC (permalink / raw)
To: Jose Abreu, Voon, Weifeng, David S. Miller, Maxime Coquelin
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Giuseppe Cavallaro, Andrew Lunn, Florian Fainelli,
Alexandre Torgue, Gomes, Vinicius
In-Reply-To: <BN8PR12MB32668CB3930DD0D9565D15B0D3FD0@BN8PR12MB3266.namprd12.prod.outlook.com>
>-----Original Message-----
>From: Jose Abreu [mailto:Jose.Abreu@synopsys.com]
>>From: Voon Weifeng <weifeng.voon@intel.com>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dw_tsn_lib.c
>b/drivers/net/ethernet/stmicro/stmmac/dw_tsn_lib.c
>> new file mode 100644
>> index 000000000000..cba27c604cb1
>> --- /dev/null
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dw_tsn_lib.c
>
>XGMAC also supports TSN features so I think more abstraction is needed
>on this because the XGMAC implementation is very similar (only reg
>offsets and bitfields changes).
>
>I would rather:
> - Implement HW specific handling in dwmac4_core.c / dwmac4_dma.c
>and
>add the callbacks in hwif table;
> - Let TSN logic in this file but call it stmmac_tsn.c.
OK. Thanks for above feedback.
>
>> @@ -3621,6 +3622,8 @@ static int stmmac_set_features(struct net_device
>*netdev,
>> */
>> stmmac_rx_ipc(priv, priv->hw);
>>
>> + netdev->features = features;
>
>Isn't this a fix ?
Yup. We will split this out as a patch and send separately.
^ permalink raw reply
* Re: linux-next: manual merge of the mlx5-next tree with the net-next tree
From: Saeed Mahameed @ 2019-06-27 22:59 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Leon Romanovsky, David Miller, Networking,
Linux Next Mailing List, Linux Kernel Mailing List,
Yevgeny Kliteynik, Saeed Mahameed, Eli Britstein, Jianbo Liu
In-Reply-To: <20190627140929.74ae7da6@canb.auug.org.au>
On Wed, Jun 26, 2019 at 9:09 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi all,
>
> Today's linux-next merge of the mlx5-next tree got a conflict in:
>
> drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
>
> between commits:
>
> 955858009708 ("net/mlx5e: Fix number of vports for ingress ACL configuration")
> d4a18e16c570 ("net/mlx5e: Enable setting multiple match criteria for flow group")
>
> from the net-next tree and commits:
>
> 7445cfb1169c ("net/mlx5: E-Switch, Tag packet with vport number in VF vports and uplink ingress ACLs")
> c01cfd0f1115 ("net/mlx5: E-Switch, Add match on vport metadata for rule in fast path")
>
> from the mlx5-next tree.
>
> I fixed it up (I basically used the latter versions) and can carry the
> fix as necessary. This is now fixed as far as linux-next is concerned,
> but any non trivial conflicts should be mentioned to your upstream
> maintainer when your tree is submitted for merging. You may also want
> to consider cooperating with the maintainer of the conflicting tree to
> minimise any particularly complex conflicts.
>
Thanks Stephen, this will be handled in my next pull request to net-next.
> --
> Cheers,
> Stephen Rothwell
^ permalink raw reply
* [PATCH net] sctp: fix error handling on stream scheduler initialization
From: Marcelo Ricardo Leitner @ 2019-06-27 22:48 UTC (permalink / raw)
To: netdev; +Cc: Xin Long, Neil Horman, linux-sctp, Hillf Danton
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
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox