[PATCH v2 net-next 0/9] bpf: xdp: Report bpf_prog ID in IFLA_XDP

[PATCH v2 net-next 0/9] bpf: xdp: Report bpf_prog ID in IFLA_XDP

[Martin KaFai Lau]

This is the first usage of the new bpf_prog ID.  It is for
reporting the ID of a xdp_prog through netlink.

It rides on the existing IFLA_XDP.  This patch adds IFLA_XDP_PROG_ID
for the bpf_prog ID reporting.

It starts with changing the generic_xdp first.  After that,
the hardware driver is changed one by one.  Jakub Kicinski mentioned
that he will soon introduce XDP_ATTACHED_HW (on top of the existing
XDP_ATTACHED_DRV and XDP_ATTACHED_SKB)
and he is going to reuse the prog_attached for this purpose.
Hence, this patch set keeps the prog_attached even though
!!prog_id also implies there is xdp_prog attached.

I have tested with generic_xdp, mlx4 and mlx5.

v2:
1. Remove READ_ONCE since it is alredy under rtnl lock
2. Keep prog_attached in 'struct netdev_xdp' as
   requested by Jakub Kicinski.  The existing prog_attached
   and the new prog_id are put under a struct for XDP_QUERY_PROG.

Martin KaFai Lau (9):
  net: Add IFLA_XDP_PROG_ID
  bpf: mlx4: Report bpf_prog ID during XDP_QUERY_PROG
  bpf: mlx5e: Report bpf_prog ID during XDP_QUERY_PROG
  bpf: virtio_net: Report bpf_prog ID during XDP_QUERY_PROG
  bpf: bnxt: Report bpf_prog ID during XDP_QUERY_PROG
  bpf: thunderx: Report bpf_prog ID during XDP_QUERY_PROG
  bpf: ixgbe: Report bpf_prog ID during XDP_QUERY_PROG
  bpf: nfp: Report bpf_prog ID during XDP_QUERY_PROG
  bpf: qede: Report bpf_prog ID during XDP_QUERY_PROG

 drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c      | 14 +++++++++--
 drivers/net/ethernet/cavium/thunder/nicvf_main.c   | 14 +++++++++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c      | 14 +++++++++--
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c     | 21 ++++++++++++++---
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c  | 15 +++++++++---
 .../net/ethernet/netronome/nfp/nfp_net_common.c    | 14 +++++++++--
 drivers/net/ethernet/qlogic/qede/qede_filter.c     | 14 +++++++++--
 drivers/net/virtio_net.c                           | 13 +++++++----
 include/linux/netdevice.h                          |  7 ++++--
 include/uapi/linux/if_link.h                       |  1 +
 net/core/dev.c                                     | 24 ++++++++++++-------
 net/core/rtnetlink.c                               | 27 +++++++++++++++++-----
 12 files changed, 141 insertions(+), 37 deletions(-)

-- 
2.9.3

[PATCH v2 net-next 1/9] net: Add IFLA_XDP_PROG_ID

[Martin KaFai Lau]

Expose prog_id through IFLA_XDP_PROG_ID.  This patch
makes modification to generic_xdp.  The later patches will
modify other xdp-supported drivers.

prog_id is added to struct net_dev_xdp.

iproute2 patch will be followed. Here is how the 'ip link'
will look like:
> ip link show eth0
3: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 xdp(prog_id:1) qdisc fq_codel state UP mode DEFAULT group default qlen 1000

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Acked-by: Alexei Starovoitov <ast@fb.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 include/linux/netdevice.h    |  7 +++++--
 include/uapi/linux/if_link.h |  1 +
 net/core/dev.c               | 24 ++++++++++++++++--------
 net/core/rtnetlink.c         | 27 +++++++++++++++++++++------
 4 files changed, 43 insertions(+), 16 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 524c7776ce96..6dc242f29eac 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -824,7 +824,10 @@ struct netdev_xdp {
 			struct netlink_ext_ack *extack;
 		};
 		/* XDP_QUERY_PROG */
-		bool prog_attached;
+		struct {
+			bool prog_attached;
+			u32 prog_id;
+		};
 	};
 };
 
@@ -3302,7 +3305,7 @@ struct sk_buff *dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
 typedef int (*xdp_op_t)(struct net_device *dev, struct netdev_xdp *xdp);
 int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
 		      int fd, u32 flags);
-bool __dev_xdp_attached(struct net_device *dev, xdp_op_t xdp_op);
+bool __dev_xdp_attached(struct net_device *dev, xdp_op_t xdp_op, u32 *prog_id);
 
 int __dev_forward_skb(struct net_device *dev, struct sk_buff *skb);
 int dev_forward_skb(struct net_device *dev, struct sk_buff *skb);
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 8ed679fe603f..dd88375a6580 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -907,6 +907,7 @@ enum {
 	IFLA_XDP_FD,
 	IFLA_XDP_ATTACHED,
 	IFLA_XDP_FLAGS,
+	IFLA_XDP_PROG_ID,
 	__IFLA_XDP_MAX,
 };
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 8f72f4a9c6ac..aa2dcb81f320 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4341,13 +4341,12 @@ static struct static_key generic_xdp_needed __read_mostly;
 
 static int generic_xdp_install(struct net_device *dev, struct netdev_xdp *xdp)
 {
+	struct bpf_prog *old = rtnl_dereference(dev->xdp_prog);
 	struct bpf_prog *new = xdp->prog;
 	int ret = 0;
 
 	switch (xdp->command) {
-	case XDP_SETUP_PROG: {
-		struct bpf_prog *old = rtnl_dereference(dev->xdp_prog);
-
+	case XDP_SETUP_PROG:
 		rcu_assign_pointer(dev->xdp_prog, new);
 		if (old)
 			bpf_prog_put(old);
@@ -4359,10 +4358,15 @@ static int generic_xdp_install(struct net_device *dev, struct netdev_xdp *xdp)
 			dev_disable_lro(dev);
 		}
 		break;
-	}
 
 	case XDP_QUERY_PROG:
-		xdp->prog_attached = !!rcu_access_pointer(dev->xdp_prog);
+		if (old) {
+			xdp->prog_attached = true;
+			xdp->prog_id = old->aux->id;
+		} else {
+			xdp->prog_attached = false;
+			xdp->prog_id = 0;
+		}
 		break;
 
 	default:
@@ -6930,7 +6934,8 @@ int dev_change_proto_down(struct net_device *dev, bool proto_down)
 }
 EXPORT_SYMBOL(dev_change_proto_down);
 
-bool __dev_xdp_attached(struct net_device *dev, xdp_op_t xdp_op)
+bool __dev_xdp_attached(struct net_device *dev, xdp_op_t xdp_op,
+			u32 *prog_id)
 {
 	struct netdev_xdp xdp;
 
@@ -6939,6 +6944,9 @@ bool __dev_xdp_attached(struct net_device *dev, xdp_op_t xdp_op)
 
 	/* Query must always succeed. */
 	WARN_ON(xdp_op(dev, &xdp) < 0);
+	if (prog_id)
+		*prog_id = xdp.prog_id;
+
 	return xdp.prog_attached;
 }
 
@@ -6984,10 +6992,10 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
 		xdp_chk = generic_xdp_install;
 
 	if (fd >= 0) {
-		if (xdp_chk && __dev_xdp_attached(dev, xdp_chk))
+		if (xdp_chk && __dev_xdp_attached(dev, xdp_chk, NULL))
 			return -EEXIST;
 		if ((flags & XDP_FLAGS_UPDATE_IF_NOEXIST) &&
-		    __dev_xdp_attached(dev, xdp_op))
+		    __dev_xdp_attached(dev, xdp_op, NULL))
 			return -EBUSY;
 
 		prog = bpf_prog_get_type(fd, BPF_PROG_TYPE_XDP);
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 7084f1db2446..f25a53b1fb92 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -39,6 +39,7 @@
 #include <linux/if_vlan.h>
 #include <linux/pci.h>
 #include <linux/etherdevice.h>
+#include <linux/bpf.h>
 
 #include <linux/uaccess.h>
 
@@ -899,7 +900,8 @@ static size_t rtnl_port_size(const struct net_device *dev,
 static size_t rtnl_xdp_size(void)
 {
 	size_t xdp_size = nla_total_size(0) +	/* nest IFLA_XDP */
-			  nla_total_size(1);	/* XDP_ATTACHED */
+			  nla_total_size(1) +	/* XDP_ATTACHED */
+			  nla_total_size(4);	/* XDP_PROG_ID */
 
 	return xdp_size;
 }
@@ -1247,15 +1249,20 @@ static int rtnl_fill_link_ifmap(struct sk_buff *skb, struct net_device *dev)
 	return 0;
 }
 
-static u8 rtnl_xdp_attached_mode(struct net_device *dev)
+static u8 rtnl_xdp_attached_mode(struct net_device *dev, u32 *prog_id)
 {
 	const struct net_device_ops *ops = dev->netdev_ops;
+	const struct bpf_prog *generic_xdp_prog;
 
 	ASSERT_RTNL();
 
-	if (rcu_access_pointer(dev->xdp_prog))
+	*prog_id = 0;
+	generic_xdp_prog = rtnl_dereference(dev->xdp_prog);
+	if (generic_xdp_prog) {
+		*prog_id = generic_xdp_prog->aux->id;
 		return XDP_ATTACHED_SKB;
-	if (ops->ndo_xdp && __dev_xdp_attached(dev, ops->ndo_xdp))
+	}
+	if (ops->ndo_xdp && __dev_xdp_attached(dev, ops->ndo_xdp, prog_id))
 		return XDP_ATTACHED_DRV;
 
 	return XDP_ATTACHED_NONE;
@@ -1264,6 +1271,7 @@ static u8 rtnl_xdp_attached_mode(struct net_device *dev)
 static int rtnl_xdp_fill(struct sk_buff *skb, struct net_device *dev)
 {
 	struct nlattr *xdp;
+	u32 prog_id;
 	int err;
 
 	xdp = nla_nest_start(skb, IFLA_XDP);
@@ -1271,10 +1279,16 @@ static int rtnl_xdp_fill(struct sk_buff *skb, struct net_device *dev)
 		return -EMSGSIZE;
 
 	err = nla_put_u8(skb, IFLA_XDP_ATTACHED,
-			 rtnl_xdp_attached_mode(dev));
+			 rtnl_xdp_attached_mode(dev, &prog_id));
 	if (err)
 		goto err_cancel;
 
+	if (prog_id) {
+		err = nla_put_u32(skb, IFLA_XDP_PROG_ID, prog_id);
+		if (err)
+			goto err_cancel;
+	}
+
 	nla_nest_end(skb, xdp);
 	return 0;
 
@@ -1552,6 +1566,7 @@ static const struct nla_policy ifla_xdp_policy[IFLA_XDP_MAX + 1] = {
 	[IFLA_XDP_FD]		= { .type = NLA_S32 },
 	[IFLA_XDP_ATTACHED]	= { .type = NLA_U8 },
 	[IFLA_XDP_FLAGS]	= { .type = NLA_U32 },
+	[IFLA_XDP_PROG_ID]	= { .type = NLA_U32 },
 };
 
 static const struct rtnl_link_ops *linkinfo_to_kind_ops(const struct nlattr *nla)
@@ -2224,7 +2239,7 @@ static int do_setlink(const struct sk_buff *skb,
 		if (err < 0)
 			goto errout;
 
-		if (xdp[IFLA_XDP_ATTACHED]) {
+		if (xdp[IFLA_XDP_ATTACHED] || xdp[IFLA_XDP_PROG_ID]) {
 			err = -EINVAL;
 			goto errout;
 		}
-- 
2.9.3

[PATCH v2 net-next 2/9] bpf: mlx4: Report bpf_prog ID during XDP_QUERY_PROG

[Martin KaFai Lau]

Add support to mlx4 to report bpf_prog ID during XDP_QUERY_PROG.

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Cc: Tariq Toukan <tariqt@mellanox.com>
Cc: Saeed Mahameed <saeedm@mellanox.com>
Acked-by: Alexei Starovoitov <ast@fb.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index c1de75fc399a..ba6ecdd505b6 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -2821,11 +2821,25 @@ static int mlx4_xdp_set(struct net_device *dev, struct bpf_prog *prog)
 	return err;
 }
 
-static bool mlx4_xdp_attached(struct net_device *dev)
+static u32 mlx4_xdp_query(struct net_device *dev)
 {
+	const struct bpf_prog *xdp_prog;
 	struct mlx4_en_priv *priv = netdev_priv(dev);
+	struct mlx4_en_dev *mdev = priv->mdev;
+	u32 prog_id = 0;
+
+	if (!priv->tx_ring_num[TX_XDP])
+		return prog_id;
+
+	mutex_lock(&mdev->state_lock);
+	xdp_prog = rcu_dereference_protected(
+		priv->rx_ring[0]->xdp_prog,
+		lockdep_is_held(&mdev->state_lock));
+	if (xdp_prog)
+		prog_id = xdp_prog->aux->id;
+	mutex_unlock(&mdev->state_lock);
 
-	return !!priv->tx_ring_num[TX_XDP];
+	return prog_id;
 }
 
 static int mlx4_xdp(struct net_device *dev, struct netdev_xdp *xdp)
@@ -2834,7 +2848,8 @@ static int mlx4_xdp(struct net_device *dev, struct netdev_xdp *xdp)
 	case XDP_SETUP_PROG:
 		return mlx4_xdp_set(dev, xdp->prog);
 	case XDP_QUERY_PROG:
-		xdp->prog_attached = mlx4_xdp_attached(dev);
+		xdp->prog_id = mlx4_xdp_query(dev);
+		xdp->prog_attached = !!xdp->prog_id;
 		return 0;
 	default:
 		return -EINVAL;
-- 
2.9.3

[PATCH v2 net-next 3/9] bpf: mlx5e: Report bpf_prog ID during XDP_QUERY_PROG

[Martin KaFai Lau]

Add support to mlx5e to report bpf_prog ID during XDP_QUERY_PROG.

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Cc: Tariq Toukan <tariqt@mellanox.com>
Cc: Saeed Mahameed <saeedm@mellanox.com>
Acked-by: Alexei Starovoitov <ast@fb.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 5afec0f4a658..1efbb1e1f78f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -3599,11 +3599,19 @@ static int mlx5e_xdp_set(struct net_device *netdev, struct bpf_prog *prog)
 	return err;
 }
 
-static bool mlx5e_xdp_attached(struct net_device *dev)
+static u32 mlx5e_xdp_query(struct net_device *dev)
 {
+	const struct bpf_prog *xdp_prog;
 	struct mlx5e_priv *priv = netdev_priv(dev);
+	u32 prog_id = 0;
 
-	return !!priv->channels.params.xdp_prog;
+	mutex_lock(&priv->state_lock);
+	xdp_prog = priv->channels.params.xdp_prog;
+	if (xdp_prog)
+		prog_id = xdp_prog->aux->id;
+	mutex_unlock(&priv->state_lock);
+
+	return prog_id;
 }
 
 static int mlx5e_xdp(struct net_device *dev, struct netdev_xdp *xdp)
@@ -3612,7 +3620,8 @@ static int mlx5e_xdp(struct net_device *dev, struct netdev_xdp *xdp)
 	case XDP_SETUP_PROG:
 		return mlx5e_xdp_set(dev, xdp->prog);
 	case XDP_QUERY_PROG:
-		xdp->prog_attached = mlx5e_xdp_attached(dev);
+		xdp->prog_id = mlx5e_xdp_query(dev);
+		xdp->prog_attached = !!xdp->prog_id;
 		return 0;
 	default:
 		return -EINVAL;
-- 
2.9.3

[PATCH v2 net-next 4/9] bpf: virtio_net: Report bpf_prog ID during XDP_QUERY_PROG

[Martin KaFai Lau]

Add support to virtio_net to report bpf_prog ID during XDP_QUERY_PROG.

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Jason Wang <jasowang@redhat.com>
Acked-by: Alexei Starovoitov <ast@fb.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 drivers/net/virtio_net.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 1f8c15cb63b0..deecd24f2db8 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1955,16 +1955,18 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 	return err;
 }
 
-static bool virtnet_xdp_query(struct net_device *dev)
+static u32 virtnet_xdp_query(struct net_device *dev)
 {
+	const struct bpf_prog *xdp_prog;
 	struct virtnet_info *vi = netdev_priv(dev);
 	int i;
 
 	for (i = 0; i < vi->max_queue_pairs; i++) {
-		if (vi->rq[i].xdp_prog)
-			return true;
+		xdp_prog = rtnl_dereference(vi->rq[i].xdp_prog);
+		if (xdp_prog)
+			return xdp_prog->aux->id;
 	}
-	return false;
+	return 0;
 }
 
 static int virtnet_xdp(struct net_device *dev, struct netdev_xdp *xdp)
@@ -1973,7 +1975,8 @@ static int virtnet_xdp(struct net_device *dev, struct netdev_xdp *xdp)
 	case XDP_SETUP_PROG:
 		return virtnet_xdp_set(dev, xdp->prog, xdp->extack);
 	case XDP_QUERY_PROG:
-		xdp->prog_attached = virtnet_xdp_query(dev);
+		xdp->prog_id = virtnet_xdp_query(dev);
+		xdp->prog_attached = !!xdp->prog_id;
 		return 0;
 	default:
 		return -EINVAL;
-- 
2.9.3

[PATCH v2 net-next 5/9] bpf: bnxt: Report bpf_prog ID during XDP_QUERY_PROG

[Martin KaFai Lau]

Add support to bnxt to report bpf_prog ID during XDP_QUERY_PROG.

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Cc: Michael Chan <michael.chan@broadcom.com>
Acked-by: Alexei Starovoitov <ast@fb.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
index 8ce793a0d030..841bc9df65b1 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
@@ -216,10 +216,20 @@ int bnxt_xdp(struct net_device *dev, struct netdev_xdp *xdp)
 	case XDP_SETUP_PROG:
 		rc = bnxt_xdp_set(bp, xdp->prog);
 		break;
-	case XDP_QUERY_PROG:
-		xdp->prog_attached = !!bp->xdp_prog;
+	case XDP_QUERY_PROG: {
+		const struct bpf_prog *xdp_prog;
+
+		xdp_prog = bp->xdp_prog;
+		if (xdp_prog) {
+			xdp->prog_id = xdp_prog->aux->id;
+			xdp->prog_attached = true;
+		} else {
+			xdp->prog_id = 0;
+			xdp->prog_attached = false;
+		}
 		rc = 0;
 		break;
+	}
 	default:
 		rc = -EINVAL;
 		break;
-- 
2.9.3

[PATCH v2 net-next 6/9] bpf: thunderx: Report bpf_prog ID during XDP_QUERY_PROG

[Martin KaFai Lau]

Add support to thunderx to report bpf_prog ID during XDP_QUERY_PROG.

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Cc: Sunil Goutham <sgoutham@cavium.com>
Acked-by: Alexei Starovoitov <ast@fb.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 drivers/net/ethernet/cavium/thunder/nicvf_main.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index d6477af88085..77096527ad06 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -1761,9 +1761,19 @@ static int nicvf_xdp(struct net_device *netdev, struct netdev_xdp *xdp)
 	switch (xdp->command) {
 	case XDP_SETUP_PROG:
 		return nicvf_xdp_setup(nic, xdp->prog);
-	case XDP_QUERY_PROG:
-		xdp->prog_attached = !!nic->xdp_prog;
+	case XDP_QUERY_PROG: {
+		const struct bpf_prog *xdp_prog;
+
+		xdp_prog = nic->xdp_prog;
+		if (xdp_prog) {
+			xdp->prog_id = xdp_prog->aux->id;
+			xdp->prog_attached = true;
+		} else {
+			xdp->prog_id = 0;
+			xdp->prog_attached = false;
+		}
 		return 0;
+	}
 	default:
 		return -EINVAL;
 	}
-- 
2.9.3

[PATCH v2 net-next 7/9] bpf: ixgbe: Report bpf_prog ID during XDP_QUERY_PROG

[Martin KaFai Lau]

Add support to ixgbe to report bpf_prog ID during XDP_QUERY_PROG.

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Cc: Alexander Duyck <alexander.h.duyck@intel.com>
Cc: John Fastabend <john.fastabend@gmail.com>
Acked-by: Alexei Starovoitov <ast@fb.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 812319ab77db..37b0e5a741d8 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -9799,9 +9799,19 @@ static int ixgbe_xdp(struct net_device *dev, struct netdev_xdp *xdp)
 	switch (xdp->command) {
 	case XDP_SETUP_PROG:
 		return ixgbe_xdp_setup(dev, xdp->prog);
-	case XDP_QUERY_PROG:
-		xdp->prog_attached = !!(adapter->xdp_prog);
+	case XDP_QUERY_PROG: {
+		const struct bpf_prog *xdp_prog;
+
+		xdp_prog = adapter->xdp_prog;
+		if (xdp_prog) {
+			xdp->prog_id = xdp_prog->aux->id;
+			xdp->prog_attached = true;
+		} else {
+			xdp->prog_id = 0;
+			xdp->prog_attached = false;
+		}
 		return 0;
+	}
 	default:
 		return -EINVAL;
 	}
-- 
2.9.3

[PATCH v2 net-next 8/9] bpf: nfp: Report bpf_prog ID during XDP_QUERY_PROG

[Martin KaFai Lau]

Add support to nfp to report bpf_prog ID during XDP_QUERY_PROG.

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Alexei Starovoitov <ast@fb.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 49d1756d6a8e..272354fb0f13 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -3254,9 +3254,19 @@ static int nfp_net_xdp(struct net_device *netdev, struct netdev_xdp *xdp)
 	switch (xdp->command) {
 	case XDP_SETUP_PROG:
 		return nfp_net_xdp_setup(nn, xdp);
-	case XDP_QUERY_PROG:
-		xdp->prog_attached = !!nn->dp.xdp_prog;
+	case XDP_QUERY_PROG: {
+		const struct bpf_prog *xdp_prog;
+
+		xdp_prog = nn->dp.xdp_prog;
+		if (xdp_prog) {
+			xdp->prog_id = xdp_prog->aux->id;
+			xdp->prog_attached = true;
+		} else {
+			xdp->prog_id = 0;
+			xdp->prog_attached = false;
+		}
 		return 0;
+	}
 	default:
 		return -EINVAL;
 	}
-- 
2.9.3

[PATCH v2 net-next 9/9] bpf: qede: Report bpf_prog ID during XDP_QUERY_PROG

[Martin KaFai Lau]

Add support to qede to report bpf_prog ID during XDP_QUERY_PROG.

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Cc: Mintz Yuval <Yuval.Mintz@cavium.com>
Acked-by: Alexei Starovoitov <ast@fb.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 drivers/net/ethernet/qlogic/qede/qede_filter.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qede/qede_filter.c b/drivers/net/ethernet/qlogic/qede/qede_filter.c
index 13955a3bd3b3..d91555ae2936 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_filter.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_filter.c
@@ -1035,9 +1035,19 @@ int qede_xdp(struct net_device *dev, struct netdev_xdp *xdp)
 	switch (xdp->command) {
 	case XDP_SETUP_PROG:
 		return qede_xdp_set(edev, xdp->prog);
-	case XDP_QUERY_PROG:
-		xdp->prog_attached = !!edev->xdp_prog;
+	case XDP_QUERY_PROG: {
+		const struct bpf_prog *xdp_prog;
+
+		xdp_prog = edev->xdp_prog;
+		if (xdp_prog) {
+			xdp->prog_id = xdp_prog->aux->id;
+			xdp->prog_attached = true;
+		} else {
+			xdp->prog_id = 0;
+			xdp->prog_attached = false;
+		}
 		return 0;
+	}
 	default:
 		return -EINVAL;
 	}
-- 
2.9.3

Re: [PATCH v2 net-next 8/9] bpf: nfp: Report bpf_prog ID during XDP_QUERY_PROG

[Jakub Kicinski]

On Tue, 13 Jun 2017 14:08:40 -0700, Martin KaFai Lau wrote:
> -	case XDP_QUERY_PROG:
> -		xdp->prog_attached = !!nn->dp.xdp_prog;
> +	case XDP_QUERY_PROG: {
> +		const struct bpf_prog *xdp_prog;
> +
> +		xdp_prog = nn->dp.xdp_prog;
> +		if (xdp_prog) {
> +			xdp->prog_id = xdp_prog->aux->id;
> +			xdp->prog_attached = true;
> +		} else {
> +			xdp->prog_id = 0;
> +			xdp->prog_attached = false;
> +		}
>  		return 0;
> +	}

I'm sorry to nit pick but it could be done on a single line:

	case XDP_QUERY_PROG:
		xdp->prog_attached = !!nn->dp.xdp_prog;
+		xdp->prog_id = nn->dp.xdp_prog ? nn->dp.xdp_prog->aux->id : 0;
 		return 0;
 	default:
 		return -EINVAL;


What would be even cooler is a helper like this:

static inline u32 bpf_prog_id(struct bpf_prog *prog)
{
	if (!prog)
		return 0;
	return prog->aux->id;
}

in linux/bpf.h.  

In patch 1 I would be tempted to add a new command for getting the prog
id, instead of muxing through query to avoid the output parameter?  But
I'm OK with the code as is, its just a preference rather than an objection :)

Re: [PATCH v2 net-next 8/9] bpf: nfp: Report bpf_prog ID during XDP_QUERY_PROG

[Martin KaFai Lau]

On Tue, Jun 13, 2017 at 04:52:32PM -0700, Jakub Kicinski wrote:
> On Tue, 13 Jun 2017 14:08:40 -0700, Martin KaFai Lau wrote:
> > -	case XDP_QUERY_PROG:
> > -		xdp->prog_attached = !!nn->dp.xdp_prog;
> > +	case XDP_QUERY_PROG: {
> > +		const struct bpf_prog *xdp_prog;
> > +
> > +		xdp_prog = nn->dp.xdp_prog;
> > +		if (xdp_prog) {
> > +			xdp->prog_id = xdp_prog->aux->id;
> > +			xdp->prog_attached = true;
> > +		} else {
> > +			xdp->prog_id = 0;
> > +			xdp->prog_attached = false;
> > +		}
> >  		return 0;
> > +	}
>
> I'm sorry to nit pick but it could be done on a single line:
>
> 	case XDP_QUERY_PROG:
> 		xdp->prog_attached = !!nn->dp.xdp_prog;
> +		xdp->prog_id = nn->dp.xdp_prog ? nn->dp.xdp_prog->aux->id : 0;
>  		return 0;
>  	default:
>  		return -EINVAL;
OK...

>
>
> What would be even cooler is a helper like this:
>
> static inline u32 bpf_prog_id(struct bpf_prog *prog)
> {
> 	if (!prog)
> 		return 0;
> 	return prog->aux->id;
> }
>
> in linux/bpf.h.
Good idea.



I had been thinking I may not need to change all the
drivers now.  I did that in v1 because I wanted to remove
prog_attached which is redundant.  With prog_attached reserved,
prog_id is optional.

Considering I don't have all the hardwares to test it,  I think
it may make more sense for me to only change the HW that I have?

>
> In patch 1 I would be tempted to add a new command for getting the prog
> id, instead of muxing through query to avoid the output parameter?  But
> I'm OK with the code as is, its just a preference rather than an objection :)
Have one command to query a new field?  I think it is overkilled.

Re: [PATCH v2 net-next 8/9] bpf: nfp: Report bpf_prog ID during XDP_QUERY_PROG

[Jakub Kicinski]

On Tue, 13 Jun 2017 17:37:50 -0700, Martin KaFai Lau wrote:
> On Tue, Jun 13, 2017 at 04:52:32PM -0700, Jakub Kicinski wrote:
> > On Tue, 13 Jun 2017 14:08:40 -0700, Martin KaFai Lau wrote:  
> > > -	case XDP_QUERY_PROG:
> > > -		xdp->prog_attached = !!nn->dp.xdp_prog;
> > > +	case XDP_QUERY_PROG: {
> > > +		const struct bpf_prog *xdp_prog;
> > > +
> > > +		xdp_prog = nn->dp.xdp_prog;
> > > +		if (xdp_prog) {
> > > +			xdp->prog_id = xdp_prog->aux->id;
> > > +			xdp->prog_attached = true;
> > > +		} else {
> > > +			xdp->prog_id = 0;
> > > +			xdp->prog_attached = false;
> > > +		}
> > >  		return 0;
> > > +	}  
> >
> > I'm sorry to nit pick but it could be done on a single line:
> >
> > 	case XDP_QUERY_PROG:
> > 		xdp->prog_attached = !!nn->dp.xdp_prog;
> > +		xdp->prog_id = nn->dp.xdp_prog ? nn->dp.xdp_prog->aux->id : 0;
> >  		return 0;
> >  	default:
> >  		return -EINVAL;  
> OK...
> 
> >
> >
> > What would be even cooler is a helper like this:
> >
> > static inline u32 bpf_prog_id(struct bpf_prog *prog)
> > {
> > 	if (!prog)
> > 		return 0;
> > 	return prog->aux->id;
> > }
> >
> > in linux/bpf.h.
> Good idea.

You may actually have to add that into a source file, because bpf.h
does not know the definition of struct bpf_prog :(

> I had been thinking I may not need to change all the
> drivers now.  I did that in v1 because I wanted to remove
> prog_attached which is redundant.  With prog_attached reserved,
> prog_id is optional.
> 
> Considering I don't have all the hardwares to test it,  I think
> it may make more sense for me to only change the HW that I have?

Coccinelle to the rescue?

@@
expression ex;
@@
		xdp->prog_attached = !!(ex);
+		xdp->prog_id = bpf_prog_id(ex);

> > In patch 1 I would be tempted to add a new command for getting the prog
> > id, instead of muxing through query to avoid the output parameter?  But
> > I'm OK with the code as is, its just a preference rather than an objection :)  
> Have one command to query a new field?  I think it is overkilled.

Perhaps.  I was just trying to come up with a way of avoiding the
output parameter.  It seems hard to do unless we stop using
__dev_xdp_attached() for filling in the netlink attributes.  I'm OK
with leaving the code as is..

Re: [PATCH v2 net-next 8/9] bpf: nfp: Report bpf_prog ID during XDP_QUERY_PROG

[Martin KaFai Lau]

On Tue, Jun 13, 2017 at 07:19:50PM -0700, Jakub Kicinski wrote:
> On Tue, 13 Jun 2017 17:37:50 -0700, Martin KaFai Lau wrote:
> > On Tue, Jun 13, 2017 at 04:52:32PM -0700, Jakub Kicinski wrote:
> > > On Tue, 13 Jun 2017 14:08:40 -0700, Martin KaFai Lau wrote:
> > > > -	case XDP_QUERY_PROG:
> > > > -		xdp->prog_attached = !!nn->dp.xdp_prog;
> > > > +	case XDP_QUERY_PROG: {
> > > > +		const struct bpf_prog *xdp_prog;
> > > > +
> > > > +		xdp_prog = nn->dp.xdp_prog;
> > > > +		if (xdp_prog) {
> > > > +			xdp->prog_id = xdp_prog->aux->id;
> > > > +			xdp->prog_attached = true;
> > > > +		} else {
> > > > +			xdp->prog_id = 0;
> > > > +			xdp->prog_attached = false;
> > > > +		}
> > > >  		return 0;
> > > > +	}
> > >
> > > I'm sorry to nit pick but it could be done on a single line:
> > >
> > > 	case XDP_QUERY_PROG:
> > > 		xdp->prog_attached = !!nn->dp.xdp_prog;
> > > +		xdp->prog_id = nn->dp.xdp_prog ? nn->dp.xdp_prog->aux->id : 0;
> > >  		return 0;
> > >  	default:
> > >  		return -EINVAL;
> > OK...
> >
> > >
> > >
> > > What would be even cooler is a helper like this:
> > >
> > > static inline u32 bpf_prog_id(struct bpf_prog *prog)
> > > {
> > > 	if (!prog)
> > > 		return 0;
> > > 	return prog->aux->id;
> > > }
> > >
> > > in linux/bpf.h.
> > Good idea.
>
> You may actually have to add that into a source file, because bpf.h
> does not know the definition of struct bpf_prog :(
Yeah. filter.h seems not working well either.
It looks good to me at the first thought.  After a second thought,
in the future, I am not so sure having a getter for every fields
in bpf_prog.

I can put bpf_prog_id() in nfp_net_common.c only.
or do 'xdp->prog_id = nn->dp.xdp_prog ? nn->dp.xdp_prog->aux->id : 0;'
as you suggested earlier also.
I am fine either way.  Your call ;)

>
> > I had been thinking I may not need to change all the
> > drivers now.  I did that in v1 because I wanted to remove
> > prog_attached which is redundant.  With prog_attached reserved,
> > prog_id is optional.
> >
> > Considering I don't have all the hardwares to test it,  I think
> > it may make more sense for me to only change the HW that I have?
>
> Coccinelle to the rescue?
>
> @@
> expression ex;
> @@
> 		xdp->prog_attached = !!(ex);
> +		xdp->prog_id = bpf_prog_id(ex);
Good to know Coccinelle.  First hit in my browser ;)

Changing it is fine.  I meant I cannot test it without the HW but
they are at least compiler tested now.

Regardless, I think I will give it one more try in v3.

Re: [PATCH v2 net-next 8/9] bpf: nfp: Report bpf_prog ID during XDP_QUERY_PROG

[Jakub Kicinski]

On Tue, 13 Jun 2017 20:19:53 -0700, Martin KaFai Lau wrote:
> On Tue, Jun 13, 2017 at 07:19:50PM -0700, Jakub Kicinski wrote:
> > On Tue, 13 Jun 2017 17:37:50 -0700, Martin KaFai Lau wrote:  
> > > On Tue, Jun 13, 2017 at 04:52:32PM -0700, Jakub Kicinski wrote:  
> > > > On Tue, 13 Jun 2017 14:08:40 -0700, Martin KaFai Lau wrote:  
> > > > > -	case XDP_QUERY_PROG:
> > > > > -		xdp->prog_attached = !!nn->dp.xdp_prog;
> > > > > +	case XDP_QUERY_PROG: {
> > > > > +		const struct bpf_prog *xdp_prog;
> > > > > +
> > > > > +		xdp_prog = nn->dp.xdp_prog;
> > > > > +		if (xdp_prog) {
> > > > > +			xdp->prog_id = xdp_prog->aux->id;
> > > > > +			xdp->prog_attached = true;
> > > > > +		} else {
> > > > > +			xdp->prog_id = 0;
> > > > > +			xdp->prog_attached = false;
> > > > > +		}
> > > > >  		return 0;
> > > > > +	}  
> > > >
> > > > I'm sorry to nit pick but it could be done on a single line:
> > > >
> > > > 	case XDP_QUERY_PROG:
> > > > 		xdp->prog_attached = !!nn->dp.xdp_prog;
> > > > +		xdp->prog_id = nn->dp.xdp_prog ? nn->dp.xdp_prog->aux->id : 0;
> > > >  		return 0;
> > > >  	default:
> > > >  		return -EINVAL;  
> > > OK...
> > >  
> > > >
> > > >
> > > > What would be even cooler is a helper like this:
> > > >
> > > > static inline u32 bpf_prog_id(struct bpf_prog *prog)
> > > > {
> > > > 	if (!prog)
> > > > 		return 0;
> > > > 	return prog->aux->id;
> > > > }
> > > >
> > > > in linux/bpf.h.  
> > > Good idea.  
> >
> > You may actually have to add that into a source file, because bpf.h
> > does not know the definition of struct bpf_prog :(  
> Yeah. filter.h seems not working well either.
> It looks good to me at the first thought.  After a second thought,
> in the future, I am not so sure having a getter for every fields
> in bpf_prog.

Yes, if it's not a static inline it's far less appealing...
 
> I can put bpf_prog_id() in nfp_net_common.c only.
> or do 'xdp->prog_id = nn->dp.xdp_prog ? nn->dp.xdp_prog->aux->id : 0;'
> as you suggested earlier also.
> I am fine either way.  Your call ;)

OK, let's do the ternary operator version then :)  Sorry for leading you
into this dead end.