* [PATCH net-next v2 01/15] net: bpf: rename ndo_xdp to ndo_bpf
2017-11-03 20:56 [PATCH net-next v2 00/15] bpf: add offload as a first class citizen Jakub Kicinski
@ 2017-11-03 20:56 ` Jakub Kicinski
2017-11-03 20:56 ` [PATCH net-next v2 02/15] bpf: offload: add infrastructure for loading programs for a specific netdev Jakub Kicinski
` (14 subsequent siblings)
15 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2017-11-03 20:56 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, Jakub Kicinski
ndo_xdp is a control path callback for setting up XDP in the
driver. We can reuse it for other forms of communication
between the eBPF stack and the drivers. Rename the callback
and associated structures and definitions.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 2 +-
drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c | 2 +-
drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.h | 2 +-
drivers/net/ethernet/cavium/thunder/nicvf_main.c | 4 +--
drivers/net/ethernet/intel/i40e/i40e_main.c | 6 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 +--
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 6 ++--
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 4 +--
.../net/ethernet/netronome/nfp/nfp_net_common.c | 4 +--
drivers/net/ethernet/qlogic/qede/qede.h | 2 +-
drivers/net/ethernet/qlogic/qede/qede_filter.c | 2 +-
drivers/net/ethernet/qlogic/qede/qede_main.c | 4 +--
drivers/net/tun.c | 4 +--
drivers/net/virtio_net.c | 4 +--
include/linux/netdevice.h | 23 ++++++++-------
net/core/dev.c | 34 +++++++++++-----------
net/core/rtnetlink.c | 4 +--
17 files changed, 56 insertions(+), 55 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 4e3d569bf32e..96416f5d97f3 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -7775,7 +7775,7 @@ static const struct net_device_ops bnxt_netdev_ops = {
#endif
.ndo_udp_tunnel_add = bnxt_udp_tunnel_add,
.ndo_udp_tunnel_del = bnxt_udp_tunnel_del,
- .ndo_xdp = bnxt_xdp,
+ .ndo_bpf = bnxt_xdp,
.ndo_bridge_getlink = bnxt_bridge_getlink,
.ndo_bridge_setlink = bnxt_bridge_setlink,
.ndo_get_phys_port_name = bnxt_get_phys_port_name
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
index 06ce63c00821..261e5847557a 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
@@ -208,7 +208,7 @@ static int bnxt_xdp_set(struct bnxt *bp, struct bpf_prog *prog)
return 0;
}
-int bnxt_xdp(struct net_device *dev, struct netdev_xdp *xdp)
+int bnxt_xdp(struct net_device *dev, struct netdev_bpf *xdp)
{
struct bnxt *bp = netdev_priv(dev);
int rc;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.h
index 12a5ad66b564..414b748038ca 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.h
@@ -16,6 +16,6 @@ void bnxt_tx_int_xdp(struct bnxt *bp, struct bnxt_napi *bnapi, int nr_pkts);
bool bnxt_rx_xdp(struct bnxt *bp, struct bnxt_rx_ring_info *rxr, u16 cons,
struct page *page, u8 **data_ptr, unsigned int *len,
u8 *event);
-int bnxt_xdp(struct net_device *dev, struct netdev_xdp *xdp);
+int bnxt_xdp(struct net_device *dev, struct netdev_bpf *xdp);
#endif
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index 71989e180289..a063c36c4c58 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -1741,7 +1741,7 @@ static int nicvf_xdp_setup(struct nicvf *nic, struct bpf_prog *prog)
return 0;
}
-static int nicvf_xdp(struct net_device *netdev, struct netdev_xdp *xdp)
+static int nicvf_xdp(struct net_device *netdev, struct netdev_bpf *xdp)
{
struct nicvf *nic = netdev_priv(netdev);
@@ -1774,7 +1774,7 @@ static const struct net_device_ops nicvf_netdev_ops = {
.ndo_tx_timeout = nicvf_tx_timeout,
.ndo_fix_features = nicvf_fix_features,
.ndo_set_features = nicvf_set_features,
- .ndo_xdp = nicvf_xdp,
+ .ndo_bpf = nicvf_xdp,
};
static int nicvf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index dfecaeda0654..05b94d87a6c3 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -11648,12 +11648,12 @@ static int i40e_xdp_setup(struct i40e_vsi *vsi,
}
/**
- * i40e_xdp - implements ndo_xdp for i40e
+ * i40e_xdp - implements ndo_bpf for i40e
* @dev: netdevice
* @xdp: XDP command
**/
static int i40e_xdp(struct net_device *dev,
- struct netdev_xdp *xdp)
+ struct netdev_bpf *xdp)
{
struct i40e_netdev_priv *np = netdev_priv(dev);
struct i40e_vsi *vsi = np->vsi;
@@ -11705,7 +11705,7 @@ static const struct net_device_ops i40e_netdev_ops = {
.ndo_features_check = i40e_features_check,
.ndo_bridge_getlink = i40e_ndo_bridge_getlink,
.ndo_bridge_setlink = i40e_ndo_bridge_setlink,
- .ndo_xdp = i40e_xdp,
+ .ndo_bpf = i40e_xdp,
};
/**
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 507977994a03..e5dcb25be398 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -10004,7 +10004,7 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
return 0;
}
-static int ixgbe_xdp(struct net_device *dev, struct netdev_xdp *xdp)
+static int ixgbe_xdp(struct net_device *dev, struct netdev_bpf *xdp)
{
struct ixgbe_adapter *adapter = netdev_priv(dev);
@@ -10113,7 +10113,7 @@ static const struct net_device_ops ixgbe_netdev_ops = {
.ndo_udp_tunnel_add = ixgbe_add_udp_tunnel_port,
.ndo_udp_tunnel_del = ixgbe_del_udp_tunnel_port,
.ndo_features_check = ixgbe_features_check,
- .ndo_xdp = ixgbe_xdp,
+ .ndo_bpf = ixgbe_xdp,
.ndo_xdp_xmit = ixgbe_xdp_xmit,
.ndo_xdp_flush = ixgbe_xdp_flush,
};
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index d611df2f274d..736a6ccaf05e 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -2916,7 +2916,7 @@ static u32 mlx4_xdp_query(struct net_device *dev)
return prog_id;
}
-static int mlx4_xdp(struct net_device *dev, struct netdev_xdp *xdp)
+static int mlx4_xdp(struct net_device *dev, struct netdev_bpf *xdp)
{
switch (xdp->command) {
case XDP_SETUP_PROG:
@@ -2958,7 +2958,7 @@ static const struct net_device_ops mlx4_netdev_ops = {
.ndo_udp_tunnel_del = mlx4_en_del_vxlan_port,
.ndo_features_check = mlx4_en_features_check,
.ndo_set_tx_maxrate = mlx4_en_set_tx_maxrate,
- .ndo_xdp = mlx4_xdp,
+ .ndo_bpf = mlx4_xdp,
};
static const struct net_device_ops mlx4_netdev_ops_master = {
@@ -2995,7 +2995,7 @@ static const struct net_device_ops mlx4_netdev_ops_master = {
.ndo_udp_tunnel_del = mlx4_en_del_vxlan_port,
.ndo_features_check = mlx4_en_features_check,
.ndo_set_tx_maxrate = mlx4_en_set_tx_maxrate,
- .ndo_xdp = mlx4_xdp,
+ .ndo_bpf = mlx4_xdp,
};
struct mlx4_en_bond {
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 28ae00b3eb88..3b7b7bb84eb0 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -3831,7 +3831,7 @@ static u32 mlx5e_xdp_query(struct net_device *dev)
return prog_id;
}
-static int mlx5e_xdp(struct net_device *dev, struct netdev_xdp *xdp)
+static int mlx5e_xdp(struct net_device *dev, struct netdev_bpf *xdp)
{
switch (xdp->command) {
case XDP_SETUP_PROG:
@@ -3883,7 +3883,7 @@ static const struct net_device_ops mlx5e_netdev_ops = {
.ndo_rx_flow_steer = mlx5e_rx_flow_steer,
#endif
.ndo_tx_timeout = mlx5e_tx_timeout,
- .ndo_xdp = mlx5e_xdp,
+ .ndo_bpf = mlx5e_xdp,
#ifdef CONFIG_NET_POLL_CONTROLLER
.ndo_poll_controller = mlx5e_netpoll,
#endif
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 185a3dd35a3f..f6c6ad4e8a59 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -3378,7 +3378,7 @@ nfp_net_xdp_setup(struct nfp_net *nn, struct bpf_prog *prog, u32 flags,
return 0;
}
-static int nfp_net_xdp(struct net_device *netdev, struct netdev_xdp *xdp)
+static int nfp_net_xdp(struct net_device *netdev, struct netdev_bpf *xdp)
{
struct nfp_net *nn = netdev_priv(netdev);
@@ -3441,7 +3441,7 @@ const struct net_device_ops nfp_net_netdev_ops = {
.ndo_get_phys_port_name = nfp_port_get_phys_port_name,
.ndo_udp_tunnel_add = nfp_net_add_vxlan_port,
.ndo_udp_tunnel_del = nfp_net_del_vxlan_port,
- .ndo_xdp = nfp_net_xdp,
+ .ndo_bpf = nfp_net_xdp,
};
/**
diff --git a/drivers/net/ethernet/qlogic/qede/qede.h b/drivers/net/ethernet/qlogic/qede/qede.h
index adb700512baa..a3a70ade411f 100644
--- a/drivers/net/ethernet/qlogic/qede/qede.h
+++ b/drivers/net/ethernet/qlogic/qede/qede.h
@@ -503,7 +503,7 @@ void qede_fill_rss_params(struct qede_dev *edev,
void qede_udp_tunnel_add(struct net_device *dev, struct udp_tunnel_info *ti);
void qede_udp_tunnel_del(struct net_device *dev, struct udp_tunnel_info *ti);
-int qede_xdp(struct net_device *dev, struct netdev_xdp *xdp);
+int qede_xdp(struct net_device *dev, struct netdev_bpf *xdp);
#ifdef CONFIG_DCB
void qede_set_dcbnl_ops(struct net_device *ndev);
diff --git a/drivers/net/ethernet/qlogic/qede/qede_filter.c b/drivers/net/ethernet/qlogic/qede/qede_filter.c
index f79e36e4060a..c1a0708a7d7c 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_filter.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_filter.c
@@ -1065,7 +1065,7 @@ static int qede_xdp_set(struct qede_dev *edev, struct bpf_prog *prog)
return 0;
}
-int qede_xdp(struct net_device *dev, struct netdev_xdp *xdp)
+int qede_xdp(struct net_device *dev, struct netdev_bpf *xdp)
{
struct qede_dev *edev = netdev_priv(dev);
diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index e5ee9f274a71..8f9b3eb82137 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -556,7 +556,7 @@ static const struct net_device_ops qede_netdev_ops = {
.ndo_udp_tunnel_add = qede_udp_tunnel_add,
.ndo_udp_tunnel_del = qede_udp_tunnel_del,
.ndo_features_check = qede_features_check,
- .ndo_xdp = qede_xdp,
+ .ndo_bpf = qede_xdp,
#ifdef CONFIG_RFS_ACCEL
.ndo_rx_flow_steer = qede_rx_flow_steer,
#endif
@@ -594,7 +594,7 @@ static const struct net_device_ops qede_netdev_vf_xdp_ops = {
.ndo_udp_tunnel_add = qede_udp_tunnel_add,
.ndo_udp_tunnel_del = qede_udp_tunnel_del,
.ndo_features_check = qede_features_check,
- .ndo_xdp = qede_xdp,
+ .ndo_bpf = qede_xdp,
};
/* -------------------------------------------------------------------------
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 8125956f62a1..1a326b697221 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1141,7 +1141,7 @@ static u32 tun_xdp_query(struct net_device *dev)
return 0;
}
-static int tun_xdp(struct net_device *dev, struct netdev_xdp *xdp)
+static int tun_xdp(struct net_device *dev, struct netdev_bpf *xdp)
{
switch (xdp->command) {
case XDP_SETUP_PROG:
@@ -1185,7 +1185,7 @@ static const struct net_device_ops tap_netdev_ops = {
.ndo_features_check = passthru_features_check,
.ndo_set_rx_headroom = tun_set_headroom,
.ndo_get_stats64 = tun_net_get_stats64,
- .ndo_xdp = tun_xdp,
+ .ndo_bpf = tun_xdp,
};
static void tun_flow_init(struct tun_struct *tun)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index fc059f193e7d..edf984406ba0 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2088,7 +2088,7 @@ static u32 virtnet_xdp_query(struct net_device *dev)
return 0;
}
-static int virtnet_xdp(struct net_device *dev, struct netdev_xdp *xdp)
+static int virtnet_xdp(struct net_device *dev, struct netdev_bpf *xdp)
{
switch (xdp->command) {
case XDP_SETUP_PROG:
@@ -2115,7 +2115,7 @@ static const struct net_device_ops virtnet_netdev = {
#ifdef CONFIG_NET_POLL_CONTROLLER
.ndo_poll_controller = virtnet_netpoll,
#endif
- .ndo_xdp = virtnet_xdp,
+ .ndo_bpf = virtnet_xdp,
.ndo_xdp_xmit = virtnet_xdp_xmit,
.ndo_xdp_flush = virtnet_xdp_flush,
.ndo_features_check = passthru_features_check,
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 7de7656550c2..9af9feaaeb64 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -779,10 +779,10 @@ enum tc_setup_type {
TC_SETUP_CBS,
};
-/* These structures hold the attributes of xdp state that are being passed
- * to the netdevice through the xdp op.
+/* These structures hold the attributes of bpf state that are being passed
+ * to the netdevice through the bpf op.
*/
-enum xdp_netdev_command {
+enum bpf_netdev_command {
/* Set or clear a bpf program used in the earliest stages of packet
* rx. The prog will have been loaded as BPF_PROG_TYPE_XDP. The callee
* is responsible for calling bpf_prog_put on any old progs that are
@@ -801,8 +801,8 @@ enum xdp_netdev_command {
struct netlink_ext_ack;
-struct netdev_xdp {
- enum xdp_netdev_command command;
+struct netdev_bpf {
+ enum bpf_netdev_command command;
union {
/* XDP_SETUP_PROG */
struct {
@@ -1124,9 +1124,10 @@ struct dev_ifalias {
* appropriate rx headroom value allows avoiding skb head copy on
* forward. Setting a negative value resets the rx headroom to the
* default value.
- * int (*ndo_xdp)(struct net_device *dev, struct netdev_xdp *xdp);
+ * int (*ndo_bpf)(struct net_device *dev, struct netdev_bpf *bpf);
* This function is used to set or query state related to XDP on the
- * netdevice. See definition of enum xdp_netdev_command for details.
+ * netdevice and manage BPF offload. See definition of
+ * enum bpf_netdev_command for details.
* int (*ndo_xdp_xmit)(struct net_device *dev, struct xdp_buff *xdp);
* This function is used to submit a XDP packet for transmit on a
* netdevice.
@@ -1315,8 +1316,8 @@ struct net_device_ops {
struct sk_buff *skb);
void (*ndo_set_rx_headroom)(struct net_device *dev,
int needed_headroom);
- int (*ndo_xdp)(struct net_device *dev,
- struct netdev_xdp *xdp);
+ int (*ndo_bpf)(struct net_device *dev,
+ struct netdev_bpf *bpf);
int (*ndo_xdp_xmit)(struct net_device *dev,
struct xdp_buff *xdp);
void (*ndo_xdp_flush)(struct net_device *dev);
@@ -3311,10 +3312,10 @@ struct sk_buff *validate_xmit_skb_list(struct sk_buff *skb, struct net_device *d
struct sk_buff *dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
struct netdev_queue *txq, int *ret);
-typedef int (*xdp_op_t)(struct net_device *dev, struct netdev_xdp *xdp);
+typedef int (*bpf_op_t)(struct net_device *dev, struct netdev_bpf *bpf);
int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
int fd, u32 flags);
-u8 __dev_xdp_attached(struct net_device *dev, xdp_op_t xdp_op, u32 *prog_id);
+u8 __dev_xdp_attached(struct net_device *dev, bpf_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/net/core/dev.c b/net/core/dev.c
index 1423cf4d695c..10cde58d3275 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4545,7 +4545,7 @@ static int __netif_receive_skb(struct sk_buff *skb)
return ret;
}
-static int generic_xdp_install(struct net_device *dev, struct netdev_xdp *xdp)
+static int generic_xdp_install(struct net_device *dev, struct netdev_bpf *xdp)
{
struct bpf_prog *old = rtnl_dereference(dev->xdp_prog);
struct bpf_prog *new = xdp->prog;
@@ -7090,26 +7090,26 @@ int dev_change_proto_down(struct net_device *dev, bool proto_down)
}
EXPORT_SYMBOL(dev_change_proto_down);
-u8 __dev_xdp_attached(struct net_device *dev, xdp_op_t xdp_op, u32 *prog_id)
+u8 __dev_xdp_attached(struct net_device *dev, bpf_op_t bpf_op, u32 *prog_id)
{
- struct netdev_xdp xdp;
+ struct netdev_bpf xdp;
memset(&xdp, 0, sizeof(xdp));
xdp.command = XDP_QUERY_PROG;
/* Query must always succeed. */
- WARN_ON(xdp_op(dev, &xdp) < 0);
+ WARN_ON(bpf_op(dev, &xdp) < 0);
if (prog_id)
*prog_id = xdp.prog_id;
return xdp.prog_attached;
}
-static int dev_xdp_install(struct net_device *dev, xdp_op_t xdp_op,
+static int dev_xdp_install(struct net_device *dev, bpf_op_t bpf_op,
struct netlink_ext_ack *extack, u32 flags,
struct bpf_prog *prog)
{
- struct netdev_xdp xdp;
+ struct netdev_bpf xdp;
memset(&xdp, 0, sizeof(xdp));
if (flags & XDP_FLAGS_HW_MODE)
@@ -7120,7 +7120,7 @@ static int dev_xdp_install(struct net_device *dev, xdp_op_t xdp_op,
xdp.flags = flags;
xdp.prog = prog;
- return xdp_op(dev, &xdp);
+ return bpf_op(dev, &xdp);
}
/**
@@ -7137,24 +7137,24 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
{
const struct net_device_ops *ops = dev->netdev_ops;
struct bpf_prog *prog = NULL;
- xdp_op_t xdp_op, xdp_chk;
+ bpf_op_t bpf_op, bpf_chk;
int err;
ASSERT_RTNL();
- xdp_op = xdp_chk = ops->ndo_xdp;
- if (!xdp_op && (flags & (XDP_FLAGS_DRV_MODE | XDP_FLAGS_HW_MODE)))
+ bpf_op = bpf_chk = ops->ndo_bpf;
+ if (!bpf_op && (flags & (XDP_FLAGS_DRV_MODE | XDP_FLAGS_HW_MODE)))
return -EOPNOTSUPP;
- if (!xdp_op || (flags & XDP_FLAGS_SKB_MODE))
- xdp_op = generic_xdp_install;
- if (xdp_op == xdp_chk)
- xdp_chk = generic_xdp_install;
+ if (!bpf_op || (flags & XDP_FLAGS_SKB_MODE))
+ bpf_op = generic_xdp_install;
+ if (bpf_op == bpf_chk)
+ bpf_chk = generic_xdp_install;
if (fd >= 0) {
- if (xdp_chk && __dev_xdp_attached(dev, xdp_chk, NULL))
+ if (bpf_chk && __dev_xdp_attached(dev, bpf_chk, NULL))
return -EEXIST;
if ((flags & XDP_FLAGS_UPDATE_IF_NOEXIST) &&
- __dev_xdp_attached(dev, xdp_op, NULL))
+ __dev_xdp_attached(dev, bpf_op, NULL))
return -EBUSY;
prog = bpf_prog_get_type(fd, BPF_PROG_TYPE_XDP);
@@ -7162,7 +7162,7 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
return PTR_ERR(prog);
}
- err = dev_xdp_install(dev, xdp_op, extack, flags, prog);
+ err = dev_xdp_install(dev, bpf_op, extack, flags, prog);
if (err < 0 && prog)
bpf_prog_put(prog);
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index de24d394c69e..1d08f5995a67 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1269,10 +1269,10 @@ static u8 rtnl_xdp_attached_mode(struct net_device *dev, u32 *prog_id)
*prog_id = generic_xdp_prog->aux->id;
return XDP_ATTACHED_SKB;
}
- if (!ops->ndo_xdp)
+ if (!ops->ndo_bpf)
return XDP_ATTACHED_NONE;
- return __dev_xdp_attached(dev, ops->ndo_xdp, prog_id);
+ return __dev_xdp_attached(dev, ops->ndo_bpf, prog_id);
}
static int rtnl_xdp_fill(struct sk_buff *skb, struct net_device *dev)
--
2.14.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH net-next v2 02/15] bpf: offload: add infrastructure for loading programs for a specific netdev
2017-11-03 20:56 [PATCH net-next v2 00/15] bpf: add offload as a first class citizen Jakub Kicinski
2017-11-03 20:56 ` [PATCH net-next v2 01/15] net: bpf: rename ndo_xdp to ndo_bpf Jakub Kicinski
@ 2017-11-03 20:56 ` Jakub Kicinski
2017-11-06 17:32 ` Daniel Borkmann
2017-11-03 20:56 ` [PATCH net-next v2 03/15] bpf: report offload info to user space Jakub Kicinski
` (13 subsequent siblings)
15 siblings, 1 reply; 23+ messages in thread
From: Jakub Kicinski @ 2017-11-03 20:56 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, Jakub Kicinski
The fact that we don't know which device the program is going
to be used on is quite limiting in current eBPF infrastructure.
We have to reverse or limit the changes which kernel makes to
the loaded bytecode if we want it to be offloaded to a networking
device. We also have to invent new APIs for debugging and
troubleshooting support.
Make it possible to load programs for a specific netdev. This
helps us to bring the debug information closer to the core
eBPF infrastructure (e.g. we will be able to reuse the verifer
log in device JIT). It allows device JITs to perform translation
on the original bytecode.
__bpf_prog_get() when called to get a reference for an attachment
point will now refuse to give it if program has a device assigned.
Following patches will add a version of that function which passes
the expected netdev in. @type argument in __bpf_prog_get() is
renamed to attach_type to make it clearer that it's only set on
attachment.
All calls to ndo_bpf are protected by rtnl, only verifier callbacks
are not. We need a wait queue to make sure netdev doesn't get
destroyed while verifier is still running and calling its driver.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
include/linux/bpf.h | 36 +++++++++
include/linux/bpf_verifier.h | 10 +++
include/linux/netdevice.h | 14 ++++
include/uapi/linux/bpf.h | 1 +
kernel/bpf/Makefile | 1 +
kernel/bpf/core.c | 10 ++-
kernel/bpf/offload.c | 182 +++++++++++++++++++++++++++++++++++++++++++
kernel/bpf/syscall.c | 17 +++-
kernel/bpf/verifier.c | 15 +++-
9 files changed, 278 insertions(+), 8 deletions(-)
create mode 100644 kernel/bpf/offload.c
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 520aeebe0d93..e45d43f9ec92 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -15,6 +15,7 @@
#include <linux/err.h>
#include <linux/rbtree_latch.h>
#include <linux/numa.h>
+#include <linux/wait.h>
struct perf_event;
struct bpf_prog;
@@ -182,6 +183,16 @@ struct bpf_verifier_ops {
struct bpf_prog *prog, u32 *target_size);
};
+struct bpf_dev_offload {
+ struct bpf_prog *prog;
+ struct net_device *netdev;
+ void *dev_priv;
+ struct list_head offloads;
+ bool dev_state;
+ bool verifier_running;
+ wait_queue_head_t verifier_done;
+};
+
struct bpf_prog_aux {
atomic_t refcnt;
u32 used_map_cnt;
@@ -199,6 +210,7 @@ struct bpf_prog_aux {
#ifdef CONFIG_SECURITY
void *security;
#endif
+ struct bpf_dev_offload *offload;
union {
struct work_struct work;
struct rcu_head rcu;
@@ -317,6 +329,7 @@ extern const struct file_operations bpf_prog_fops;
#undef BPF_PROG_TYPE
#undef BPF_MAP_TYPE
+extern const struct bpf_prog_ops bpf_offload_prog_ops;
extern const struct bpf_verifier_ops tc_cls_act_analyzer_ops;
extern const struct bpf_verifier_ops xdp_analyzer_ops;
@@ -491,6 +504,29 @@ static inline int cpu_map_enqueue(struct bpf_cpu_map_entry *rcpu,
}
#endif /* CONFIG_BPF_SYSCALL */
+int bpf_prog_offload_compile(struct bpf_prog *prog);
+void bpf_prog_offload_destroy(struct bpf_prog *prog);
+
+#if defined(CONFIG_NET) && defined(CONFIG_BPF_SYSCALL)
+int bpf_prog_offload_init(struct bpf_prog *prog, union bpf_attr *attr);
+
+static inline bool bpf_prog_is_dev_bound(struct bpf_prog_aux *aux)
+{
+ return aux->offload;
+}
+#else
+static inline int bpf_prog_offload_init(struct bpf_prog *prog,
+ union bpf_attr *attr)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline bool bpf_prog_is_dev_bound(struct bpf_prog_aux *aux)
+{
+ return false;
+}
+#endif /* CONFIG_NET && CONFIG_BPF_SYSCALL */
+
#if defined(CONFIG_STREAM_PARSER) && defined(CONFIG_BPF_SYSCALL)
struct sock *__sock_map_lookup_elem(struct bpf_map *map, u32 key);
int sock_map_prog(struct bpf_map *map, struct bpf_prog *prog, u32 type);
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 3b0976aaac75..e45011dbc02d 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -153,6 +153,7 @@ struct bpf_verifier_env {
struct bpf_verifier_state *cur_state; /* current verifier state */
struct bpf_verifier_state_list **explored_states; /* search pruning optimization */
const struct bpf_ext_analyzer_ops *analyzer_ops; /* external analyzer ops */
+ const struct bpf_ext_analyzer_ops *dev_ops; /* device analyzer ops */
void *analyzer_priv; /* pointer to external analyzer's private data */
struct bpf_map *used_maps[MAX_USED_MAPS]; /* array of map's used by eBPF program */
u32 used_map_cnt; /* number of used maps */
@@ -169,6 +170,15 @@ static inline struct bpf_reg_state *cur_regs(struct bpf_verifier_env *env)
return env->cur_state->regs;
}
+#if defined(CONFIG_NET) && defined(CONFIG_BPF_SYSCALL)
+int bpf_prog_offload_verifier_prep(struct bpf_verifier_env *env);
+#else
+int bpf_prog_offload_verifier_prep(struct bpf_verifier_env *env)
+{
+ return -EOPNOTSUPP;
+}
+#endif
+
int bpf_analyzer(struct bpf_prog *prog, const struct bpf_ext_analyzer_ops *ops,
void *priv);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 9af9feaaeb64..fda527ccb263 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -797,8 +797,13 @@ enum bpf_netdev_command {
* is equivalent to XDP_ATTACHED_DRV.
*/
XDP_QUERY_PROG,
+ /* BPF program for offload callbacks, invoked at program load time. */
+ BPF_OFFLOAD_VERIFIER_PREP,
+ BPF_OFFLOAD_TRANSLATE,
+ BPF_OFFLOAD_DESTROY,
};
+struct bpf_ext_analyzer_ops;
struct netlink_ext_ack;
struct netdev_bpf {
@@ -815,6 +820,15 @@ struct netdev_bpf {
u8 prog_attached;
u32 prog_id;
};
+ /* BPF_OFFLOAD_VERIFIER_PREP */
+ struct {
+ struct bpf_prog *prog;
+ const struct bpf_ext_analyzer_ops *ops; /* callee set */
+ } verifier;
+ /* BPF_OFFLOAD_TRANSLATE, BPF_OFFLOAD_DESTROY */
+ struct {
+ struct bpf_prog *prog;
+ } offload;
};
};
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 7cebba491011..727a3dba13e6 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -259,6 +259,7 @@ union bpf_attr {
__u32 kern_version; /* checked when prog_type=kprobe */
__u32 prog_flags;
char prog_name[BPF_OBJ_NAME_LEN];
+ __u32 prog_target_ifindex; /* ifindex of netdev to prep for */
};
struct { /* anonymous struct used by BPF_OBJ_* commands */
diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
index e597daae6120..05fef663ba16 100644
--- a/kernel/bpf/Makefile
+++ b/kernel/bpf/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_BPF_SYSCALL) += disasm.o
ifeq ($(CONFIG_NET),y)
obj-$(CONFIG_BPF_SYSCALL) += devmap.o
obj-$(CONFIG_BPF_SYSCALL) += cpumap.o
+obj-$(CONFIG_BPF_SYSCALL) += offload.o
ifeq ($(CONFIG_STREAM_PARSER),y)
obj-$(CONFIG_BPF_SYSCALL) += sockmap.o
endif
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 7fe448799d76..8a6c37762330 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1380,7 +1380,13 @@ struct bpf_prog *bpf_prog_select_runtime(struct bpf_prog *fp, int *err)
* valid program, which in this case would simply not
* be JITed, but falls back to the interpreter.
*/
- fp = bpf_int_jit_compile(fp);
+ if (!bpf_prog_is_dev_bound(fp->aux)) {
+ fp = bpf_int_jit_compile(fp);
+ } else {
+ *err = bpf_prog_offload_compile(fp);
+ if (*err)
+ return fp;
+ }
bpf_prog_lock_ro(fp);
/* The tail call compatibility check can only be done at
@@ -1549,6 +1555,8 @@ static void bpf_prog_free_deferred(struct work_struct *work)
struct bpf_prog_aux *aux;
aux = container_of(work, struct bpf_prog_aux, work);
+ if (bpf_prog_is_dev_bound(aux))
+ bpf_prog_offload_destroy(aux->prog);
bpf_jit_free(aux->prog);
}
diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c
new file mode 100644
index 000000000000..5553e0e2f8b1
--- /dev/null
+++ b/kernel/bpf/offload.c
@@ -0,0 +1,182 @@
+#include <linux/bpf.h>
+#include <linux/bpf_verifier.h>
+#include <linux/bug.h>
+#include <linux/list.h>
+#include <linux/netdevice.h>
+#include <linux/printk.h>
+#include <linux/rtnetlink.h>
+
+/* protected by RTNL */
+static LIST_HEAD(bpf_prog_offload_devs);
+
+int bpf_prog_offload_init(struct bpf_prog *prog, union bpf_attr *attr)
+{
+ struct net *net = current->nsproxy->net_ns;
+ struct bpf_dev_offload *offload;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ if (attr->prog_flags)
+ return -EINVAL;
+
+ offload = kzalloc(sizeof(*offload), GFP_USER);
+ if (!offload)
+ return -ENOMEM;
+
+ offload->prog = prog;
+ init_waitqueue_head(&offload->verifier_done);
+
+ rtnl_lock();
+ offload->netdev = __dev_get_by_index(net, attr->prog_target_ifindex);
+ if (!offload->netdev) {
+ rtnl_unlock();
+ kfree(offload);
+ return -EINVAL;
+ }
+
+ prog->aux->offload = offload;
+ list_add_tail(&offload->offloads, &bpf_prog_offload_devs);
+ rtnl_unlock();
+
+ return 0;
+}
+
+static int __bpf_offload_ndo(struct bpf_prog *prog, enum bpf_netdev_command cmd,
+ struct netdev_bpf *data)
+{
+ struct net_device *netdev = prog->aux->offload->netdev;
+
+ ASSERT_RTNL();
+
+ if (!netdev)
+ return -ENODEV;
+ if (!netdev->netdev_ops->ndo_bpf)
+ return -EOPNOTSUPP;
+
+ data->command = cmd;
+
+ return netdev->netdev_ops->ndo_bpf(netdev, data);
+}
+
+int bpf_prog_offload_verifier_prep(struct bpf_verifier_env *env)
+{
+ struct netdev_bpf data = {};
+ int err;
+
+ data.verifier.prog = env->prog;
+
+ rtnl_lock();
+ err = __bpf_offload_ndo(env->prog, BPF_OFFLOAD_VERIFIER_PREP, &data);
+ if (err)
+ goto exit_unlock;
+
+ env->dev_ops = data.verifier.ops;
+
+ env->prog->aux->offload->dev_state = true;
+ env->prog->aux->offload->verifier_running = true;
+exit_unlock:
+ rtnl_unlock();
+ return err;
+}
+
+static void __bpf_prog_offload_destroy(struct bpf_prog *prog)
+{
+ struct bpf_dev_offload *offload = prog->aux->offload;
+ struct netdev_bpf data = {};
+
+ data.offload.prog = prog;
+
+ if (offload->verifier_running)
+ wait_event(offload->verifier_done, !offload->verifier_running);
+
+ if (offload->dev_state)
+ WARN_ON(__bpf_offload_ndo(prog, BPF_OFFLOAD_DESTROY, &data));
+
+ offload->dev_state = false;
+ list_del_init(&offload->offloads);
+ offload->netdev = NULL;
+}
+
+void bpf_prog_offload_destroy(struct bpf_prog *prog)
+{
+ struct bpf_dev_offload *offload = prog->aux->offload;
+
+ offload->verifier_running = false;
+ wake_up(&offload->verifier_done);
+
+ rtnl_lock();
+ __bpf_prog_offload_destroy(prog);
+ rtnl_unlock();
+
+ kfree(offload);
+}
+
+static int bpf_prog_offload_translate(struct bpf_prog *prog)
+{
+ struct bpf_dev_offload *offload = prog->aux->offload;
+ struct netdev_bpf data = {};
+ int ret;
+
+ data.offload.prog = prog;
+
+ offload->verifier_running = false;
+ wake_up(&offload->verifier_done);
+
+ rtnl_lock();
+ ret = __bpf_offload_ndo(prog, BPF_OFFLOAD_TRANSLATE, &data);
+ rtnl_unlock();
+
+ return ret;
+}
+
+static unsigned int bpf_prog_warn_on_exec(const void *ctx,
+ const struct bpf_insn *insn)
+{
+ WARN(1, "attempt to execute device eBPF program on the host!");
+ return 0;
+}
+
+int bpf_prog_offload_compile(struct bpf_prog *prog)
+{
+ prog->bpf_func = bpf_prog_warn_on_exec;
+
+ return bpf_prog_offload_translate(prog);
+}
+
+const struct bpf_prog_ops bpf_offload_prog_ops = {
+};
+
+static int bpf_offload_notification(struct notifier_block *notifier,
+ ulong event, void *ptr)
+{
+ struct net_device *netdev = netdev_notifier_info_to_dev(ptr);
+ struct bpf_dev_offload *offload, *tmp;
+
+ ASSERT_RTNL();
+
+ switch (event) {
+ case NETDEV_UNREGISTER:
+ list_for_each_entry_safe(offload, tmp, &bpf_prog_offload_devs,
+ offloads) {
+ if (offload->netdev == netdev)
+ __bpf_prog_offload_destroy(offload->prog);
+ }
+ break;
+ default:
+ break;
+ }
+ return NOTIFY_OK;
+}
+
+static struct notifier_block bpf_offload_notifier = {
+ .notifier_call = bpf_offload_notification,
+};
+
+static int __init bpf_offload_init(void)
+{
+ register_netdevice_notifier(&bpf_offload_notifier);
+ return 0;
+}
+
+subsys_initcall(bpf_offload_init);
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 323be2473c4b..1574b9f0f24e 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -824,7 +824,10 @@ static int find_prog_type(enum bpf_prog_type type, struct bpf_prog *prog)
if (type >= ARRAY_SIZE(bpf_prog_types) || !bpf_prog_types[type])
return -EINVAL;
- prog->aux->ops = bpf_prog_types[type];
+ if (!bpf_prog_is_dev_bound(prog->aux))
+ prog->aux->ops = bpf_prog_types[type];
+ else
+ prog->aux->ops = &bpf_offload_prog_ops;
prog->type = type;
return 0;
}
@@ -1054,7 +1057,7 @@ struct bpf_prog *bpf_prog_inc_not_zero(struct bpf_prog *prog)
}
EXPORT_SYMBOL_GPL(bpf_prog_inc_not_zero);
-static struct bpf_prog *__bpf_prog_get(u32 ufd, enum bpf_prog_type *type)
+static struct bpf_prog *__bpf_prog_get(u32 ufd, enum bpf_prog_type *attach_type)
{
struct fd f = fdget(ufd);
struct bpf_prog *prog;
@@ -1062,7 +1065,7 @@ static struct bpf_prog *__bpf_prog_get(u32 ufd, enum bpf_prog_type *type)
prog = ____bpf_prog_get(f);
if (IS_ERR(prog))
return prog;
- if (type && prog->type != *type) {
+ if (attach_type && (prog->type != *attach_type || prog->aux->offload)) {
prog = ERR_PTR(-EINVAL);
goto out;
}
@@ -1089,7 +1092,7 @@ struct bpf_prog *bpf_prog_get_type(u32 ufd, enum bpf_prog_type type)
EXPORT_SYMBOL_GPL(bpf_prog_get_type);
/* last field in 'union bpf_attr' used by this command */
-#define BPF_PROG_LOAD_LAST_FIELD prog_name
+#define BPF_PROG_LOAD_LAST_FIELD prog_target_ifindex
static int bpf_prog_load(union bpf_attr *attr)
{
@@ -1152,6 +1155,12 @@ static int bpf_prog_load(union bpf_attr *attr)
atomic_set(&prog->aux->refcnt, 1);
prog->gpl_compatible = is_gpl ? 1 : 0;
+ if (attr->prog_target_ifindex) {
+ err = bpf_prog_offload_init(prog, attr);
+ if (err)
+ goto free_prog;
+ }
+
/* find program type: socket_filter vs tracing_filter */
err = find_prog_type(type, prog);
if (err < 0)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 04357ad5a812..51aabb32ad67 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3736,10 +3736,13 @@ static int is_state_visited(struct bpf_verifier_env *env, int insn_idx)
static int ext_analyzer_insn_hook(struct bpf_verifier_env *env,
int insn_idx, int prev_insn_idx)
{
- if (!env->analyzer_ops || !env->analyzer_ops->insn_hook)
- return 0;
+ if (env->analyzer_ops && env->analyzer_ops->insn_hook)
+ return env->analyzer_ops->insn_hook(env, insn_idx,
+ prev_insn_idx);
+ if (env->dev_ops && env->dev_ops->insn_hook)
+ return env->dev_ops->insn_hook(env, insn_idx, prev_insn_idx);
- return env->analyzer_ops->insn_hook(env, insn_idx, prev_insn_idx);
+ return 0;
}
static int do_check(struct bpf_verifier_env *env)
@@ -4516,6 +4519,12 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr)
if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS))
env->strict_alignment = true;
+ if (env->prog->aux->offload) {
+ ret = bpf_prog_offload_verifier_prep(env);
+ if (ret)
+ goto err_unlock;
+ }
+
ret = replace_map_fd_with_map_ptr(env);
if (ret < 0)
goto skip_full_check;
--
2.14.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH net-next v2 02/15] bpf: offload: add infrastructure for loading programs for a specific netdev
2017-11-03 20:56 ` [PATCH net-next v2 02/15] bpf: offload: add infrastructure for loading programs for a specific netdev Jakub Kicinski
@ 2017-11-06 17:32 ` Daniel Borkmann
2017-11-10 1:58 ` Jakub Kicinski
0 siblings, 1 reply; 23+ messages in thread
From: Daniel Borkmann @ 2017-11-06 17:32 UTC (permalink / raw)
To: Jakub Kicinski, netdev; +Cc: oss-drivers, alexei.starovoitov
On 11/03/2017 09:56 PM, Jakub Kicinski wrote:
> The fact that we don't know which device the program is going
> to be used on is quite limiting in current eBPF infrastructure.
> We have to reverse or limit the changes which kernel makes to
> the loaded bytecode if we want it to be offloaded to a networking
> device. We also have to invent new APIs for debugging and
> troubleshooting support.
>
> Make it possible to load programs for a specific netdev. This
> helps us to bring the debug information closer to the core
> eBPF infrastructure (e.g. we will be able to reuse the verifer
> log in device JIT). It allows device JITs to perform translation
> on the original bytecode.
>
> __bpf_prog_get() when called to get a reference for an attachment
> point will now refuse to give it if program has a device assigned.
> Following patches will add a version of that function which passes
> the expected netdev in. @type argument in __bpf_prog_get() is
> renamed to attach_type to make it clearer that it's only set on
> attachment.
>
> All calls to ndo_bpf are protected by rtnl, only verifier callbacks
> are not. We need a wait queue to make sure netdev doesn't get
> destroyed while verifier is still running and calling its driver.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Simon Horman <simon.horman@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
First of all, great work, I went over the series and I really like
the outcome. It's applied already anyway, but two minor comments
further below.
[...]
> @@ -1549,6 +1555,8 @@ static void bpf_prog_free_deferred(struct work_struct *work)
> struct bpf_prog_aux *aux;
>
> aux = container_of(work, struct bpf_prog_aux, work);
> + if (bpf_prog_is_dev_bound(aux))
> + bpf_prog_offload_destroy(aux->prog);
> bpf_jit_free(aux->prog);
> }
[...]
> +static int bpf_offload_notification(struct notifier_block *notifier,
> + ulong event, void *ptr)
> +{
> + struct net_device *netdev = netdev_notifier_info_to_dev(ptr);
> + struct bpf_dev_offload *offload, *tmp;
> +
> + ASSERT_RTNL();
> +
> + switch (event) {
> + case NETDEV_UNREGISTER:
> + list_for_each_entry_safe(offload, tmp, &bpf_prog_offload_devs,
> + offloads) {
> + if (offload->netdev == netdev)
> + __bpf_prog_offload_destroy(offload->prog);
We would be calling this twice, right? Once here and then on prog
destruction again. __bpf_prog_offload_destroy() looks it will handle
this just fine, but we should probably add a comment to
__bpf_prog_offload_destroy() such that when changes are made to it
it's obvious that we need to be extra careful.
[...]
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 323be2473c4b..1574b9f0f24e 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -824,7 +824,10 @@ static int find_prog_type(enum bpf_prog_type type, struct bpf_prog *prog)
> if (type >= ARRAY_SIZE(bpf_prog_types) || !bpf_prog_types[type])
> return -EINVAL;
>
> - prog->aux->ops = bpf_prog_types[type];
> + if (!bpf_prog_is_dev_bound(prog->aux))
> + prog->aux->ops = bpf_prog_types[type];
> + else
> + prog->aux->ops = &bpf_offload_prog_ops;
> prog->type = type;
> return 0;
> }
> @@ -1054,7 +1057,7 @@ struct bpf_prog *bpf_prog_inc_not_zero(struct bpf_prog *prog)
> }
> EXPORT_SYMBOL_GPL(bpf_prog_inc_not_zero);
>
> -static struct bpf_prog *__bpf_prog_get(u32 ufd, enum bpf_prog_type *type)
> +static struct bpf_prog *__bpf_prog_get(u32 ufd, enum bpf_prog_type *attach_type)
> {
> struct fd f = fdget(ufd);
> struct bpf_prog *prog;
> @@ -1062,7 +1065,7 @@ static struct bpf_prog *__bpf_prog_get(u32 ufd, enum bpf_prog_type *type)
> prog = ____bpf_prog_get(f);
> if (IS_ERR(prog))
> return prog;
> - if (type && prog->type != *type) {
> + if (attach_type && (prog->type != *attach_type || prog->aux->offload)) {
> prog = ERR_PTR(-EINVAL);
> goto out;
> }
> @@ -1089,7 +1092,7 @@ struct bpf_prog *bpf_prog_get_type(u32 ufd, enum bpf_prog_type type)
> EXPORT_SYMBOL_GPL(bpf_prog_get_type);
>
> /* last field in 'union bpf_attr' used by this command */
> -#define BPF_PROG_LOAD_LAST_FIELD prog_name
> +#define BPF_PROG_LOAD_LAST_FIELD prog_target_ifindex
For program types that are neither XDP nor cls_bpf, we should reject
the request if something calls bpf(2) with non-0 prog_target_ifindex.
That way, i) we don't burn the whole field and could perhaps reuse/union
it for other prog types like tracing in future. Probably makes sense to
do anyway since ii) for types like tracing, we would want to reject this
upfront here and not when later attach happens.
I probably missed something when reading the code, but if I spotted
that correctly, we might otherwise even go and nfp-jit simple progs
for non-networking types (we would bail out later though on in
__bpf_prog_get() ... but we shouldn't let syscall return in first
place)?
> static int bpf_prog_load(union bpf_attr *attr)
> {
> @@ -1152,6 +1155,12 @@ static int bpf_prog_load(union bpf_attr *attr)
> atomic_set(&prog->aux->refcnt, 1);
> prog->gpl_compatible = is_gpl ? 1 : 0;
>
> + if (attr->prog_target_ifindex) {
> + err = bpf_prog_offload_init(prog, attr);
> + if (err)
> + goto free_prog;
> + }
> +
> /* find program type: socket_filter vs tracing_filter */
> err = find_prog_type(type, prog);
[...]
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH net-next v2 02/15] bpf: offload: add infrastructure for loading programs for a specific netdev
2017-11-06 17:32 ` Daniel Borkmann
@ 2017-11-10 1:58 ` Jakub Kicinski
0 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2017-11-10 1:58 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: netdev, oss-drivers, alexei.starovoitov
Hi!
Sorry for the delay!
On Mon, 06 Nov 2017 18:32:45 +0100, Daniel Borkmann wrote:
> On 11/03/2017 09:56 PM, Jakub Kicinski wrote:
> > @@ -1549,6 +1555,8 @@ static void bpf_prog_free_deferred(struct work_struct *work)
> > struct bpf_prog_aux *aux;
> >
> > aux = container_of(work, struct bpf_prog_aux, work);
> > + if (bpf_prog_is_dev_bound(aux))
> > + bpf_prog_offload_destroy(aux->prog);
> > bpf_jit_free(aux->prog);
> > }
> [...]
> > +static int bpf_offload_notification(struct notifier_block *notifier,
> > + ulong event, void *ptr)
> > +{
> > + struct net_device *netdev = netdev_notifier_info_to_dev(ptr);
> > + struct bpf_dev_offload *offload, *tmp;
> > +
> > + ASSERT_RTNL();
> > +
> > + switch (event) {
> > + case NETDEV_UNREGISTER:
> > + list_for_each_entry_safe(offload, tmp, &bpf_prog_offload_devs,
> > + offloads) {
> > + if (offload->netdev == netdev)
> > + __bpf_prog_offload_destroy(offload->prog);
>
> We would be calling this twice, right? Once here and then on prog
> destruction again. __bpf_prog_offload_destroy() looks it will handle
> this just fine, but we should probably add a comment to
> __bpf_prog_offload_destroy() such that when changes are made to it
> it's obvious that we need to be extra careful.
Good point, I will add the comment.
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index 323be2473c4b..1574b9f0f24e 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -824,7 +824,10 @@ static int find_prog_type(enum bpf_prog_type type, struct bpf_prog *prog)
> > if (type >= ARRAY_SIZE(bpf_prog_types) || !bpf_prog_types[type])
> > return -EINVAL;
> >
> > - prog->aux->ops = bpf_prog_types[type];
> > + if (!bpf_prog_is_dev_bound(prog->aux))
> > + prog->aux->ops = bpf_prog_types[type];
> > + else
> > + prog->aux->ops = &bpf_offload_prog_ops;
> > prog->type = type;
> > return 0;
> > }
> > @@ -1054,7 +1057,7 @@ struct bpf_prog *bpf_prog_inc_not_zero(struct bpf_prog *prog)
> > }
> > EXPORT_SYMBOL_GPL(bpf_prog_inc_not_zero);
> >
> > -static struct bpf_prog *__bpf_prog_get(u32 ufd, enum bpf_prog_type *type)
> > +static struct bpf_prog *__bpf_prog_get(u32 ufd, enum bpf_prog_type *attach_type)
> > {
> > struct fd f = fdget(ufd);
> > struct bpf_prog *prog;
> > @@ -1062,7 +1065,7 @@ static struct bpf_prog *__bpf_prog_get(u32 ufd, enum bpf_prog_type *type)
> > prog = ____bpf_prog_get(f);
> > if (IS_ERR(prog))
> > return prog;
> > - if (type && prog->type != *type) {
> > + if (attach_type && (prog->type != *attach_type || prog->aux->offload)) {
> > prog = ERR_PTR(-EINVAL);
> > goto out;
> > }
> > @@ -1089,7 +1092,7 @@ struct bpf_prog *bpf_prog_get_type(u32 ufd, enum bpf_prog_type type)
> > EXPORT_SYMBOL_GPL(bpf_prog_get_type);
> >
> > /* last field in 'union bpf_attr' used by this command */
> > -#define BPF_PROG_LOAD_LAST_FIELD prog_name
> > +#define BPF_PROG_LOAD_LAST_FIELD prog_target_ifindex
>
> For program types that are neither XDP nor cls_bpf, we should reject
> the request if something calls bpf(2) with non-0 prog_target_ifindex.
>
> That way, i) we don't burn the whole field and could perhaps reuse/union
> it for other prog types like tracing in future. Probably makes sense to
> do anyway since ii) for types like tracing, we would want to reject this
> upfront here and not when later attach happens.
>
> I probably missed something when reading the code, but if I spotted
> that correctly, we might otherwise even go and nfp-jit simple progs
> for non-networking types (we would bail out later though on in
> __bpf_prog_get() ... but we shouldn't let syscall return in first
> place)?
Agreed, I will fix this.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH net-next v2 03/15] bpf: report offload info to user space
2017-11-03 20:56 [PATCH net-next v2 00/15] bpf: add offload as a first class citizen Jakub Kicinski
2017-11-03 20:56 ` [PATCH net-next v2 01/15] net: bpf: rename ndo_xdp to ndo_bpf Jakub Kicinski
2017-11-03 20:56 ` [PATCH net-next v2 02/15] bpf: offload: add infrastructure for loading programs for a specific netdev Jakub Kicinski
@ 2017-11-03 20:56 ` Jakub Kicinski
2017-11-04 9:45 ` Alexei Starovoitov
2017-11-03 20:56 ` [PATCH net-next v2 04/15] bpftool: print program device bound info Jakub Kicinski
` (12 subsequent siblings)
15 siblings, 1 reply; 23+ messages in thread
From: Jakub Kicinski @ 2017-11-03 20:56 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, Jakub Kicinski
Extend struct bpf_prog_info to contain information about program
being bound to a device. Since the netdev may get destroyed while
program still exists we need a flag to indicate the program is
loaded for a device, even if the device is gone.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
include/linux/bpf.h | 1 +
include/uapi/linux/bpf.h | 6 ++++++
kernel/bpf/offload.c | 12 ++++++++++++
kernel/bpf/syscall.c | 5 +++++
4 files changed, 24 insertions(+)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index e45d43f9ec92..98bacd0fa5cc 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -506,6 +506,7 @@ static inline int cpu_map_enqueue(struct bpf_cpu_map_entry *rcpu,
int bpf_prog_offload_compile(struct bpf_prog *prog);
void bpf_prog_offload_destroy(struct bpf_prog *prog);
+u32 bpf_prog_offload_ifindex(struct bpf_prog *prog);
#if defined(CONFIG_NET) && defined(CONFIG_BPF_SYSCALL)
int bpf_prog_offload_init(struct bpf_prog *prog, union bpf_attr *attr);
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 727a3dba13e6..e92f62cf933a 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -894,6 +894,10 @@ enum sk_action {
#define BPF_TAG_SIZE 8
+enum bpf_prog_status {
+ BPF_PROG_STATUS_DEV_BOUND = (1 << 0),
+};
+
struct bpf_prog_info {
__u32 type;
__u32 id;
@@ -907,6 +911,8 @@ struct bpf_prog_info {
__u32 nr_map_ids;
__aligned_u64 map_ids;
char name[BPF_OBJ_NAME_LEN];
+ __u32 ifindex;
+ __u32 status;
} __attribute__((aligned(8)));
struct bpf_map_info {
diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c
index 5553e0e2f8b1..2816feb38be1 100644
--- a/kernel/bpf/offload.c
+++ b/kernel/bpf/offload.c
@@ -144,6 +144,18 @@ int bpf_prog_offload_compile(struct bpf_prog *prog)
return bpf_prog_offload_translate(prog);
}
+u32 bpf_prog_offload_ifindex(struct bpf_prog *prog)
+{
+ struct bpf_dev_offload *offload = prog->aux->offload;
+ u32 ifindex;
+
+ rtnl_lock();
+ ifindex = offload->netdev ? offload->netdev->ifindex : 0;
+ rtnl_unlock();
+
+ return ifindex;
+}
+
const struct bpf_prog_ops bpf_offload_prog_ops = {
};
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 1574b9f0f24e..3217c20ea91b 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1592,6 +1592,11 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
return -EFAULT;
}
+ if (bpf_prog_is_dev_bound(prog->aux)) {
+ info.status |= BPF_PROG_STATUS_DEV_BOUND;
+ info.ifindex = bpf_prog_offload_ifindex(prog);
+ }
+
done:
if (copy_to_user(uinfo, &info, info_len) ||
put_user(info_len, &uattr->info.info_len))
--
2.14.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH net-next v2 03/15] bpf: report offload info to user space
2017-11-03 20:56 ` [PATCH net-next v2 03/15] bpf: report offload info to user space Jakub Kicinski
@ 2017-11-04 9:45 ` Alexei Starovoitov
2017-11-04 10:32 ` Jakub Kicinski
0 siblings, 1 reply; 23+ messages in thread
From: Alexei Starovoitov @ 2017-11-04 9:45 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, oss-drivers, daniel
On Fri, Nov 03, 2017 at 01:56:18PM -0700, Jakub Kicinski wrote:
> Extend struct bpf_prog_info to contain information about program
> being bound to a device. Since the netdev may get destroyed while
> program still exists we need a flag to indicate the program is
> loaded for a device, even if the device is gone.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Simon Horman <simon.horman@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
> ---
> include/linux/bpf.h | 1 +
> include/uapi/linux/bpf.h | 6 ++++++
> kernel/bpf/offload.c | 12 ++++++++++++
> kernel/bpf/syscall.c | 5 +++++
> 4 files changed, 24 insertions(+)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index e45d43f9ec92..98bacd0fa5cc 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -506,6 +506,7 @@ static inline int cpu_map_enqueue(struct bpf_cpu_map_entry *rcpu,
>
> int bpf_prog_offload_compile(struct bpf_prog *prog);
> void bpf_prog_offload_destroy(struct bpf_prog *prog);
> +u32 bpf_prog_offload_ifindex(struct bpf_prog *prog);
>
> #if defined(CONFIG_NET) && defined(CONFIG_BPF_SYSCALL)
> int bpf_prog_offload_init(struct bpf_prog *prog, union bpf_attr *attr);
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 727a3dba13e6..e92f62cf933a 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -894,6 +894,10 @@ enum sk_action {
>
> #define BPF_TAG_SIZE 8
>
> +enum bpf_prog_status {
> + BPF_PROG_STATUS_DEV_BOUND = (1 << 0),
> +};
> +
> struct bpf_prog_info {
> __u32 type;
> __u32 id;
> @@ -907,6 +911,8 @@ struct bpf_prog_info {
> __u32 nr_map_ids;
> __aligned_u64 map_ids;
> char name[BPF_OBJ_NAME_LEN];
> + __u32 ifindex;
> + __u32 status;
why status is needed?
ifindex cannot be zero, so if it's set > 0 would mean
that the program is bound.
Also would be good to have consistent name with prog_load.
imo prog_target_ifindex is too long.
May be call it 'ifindex' both in bpf_attr and in bpf_prog_info ?
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH net-next v2 03/15] bpf: report offload info to user space
2017-11-04 9:45 ` Alexei Starovoitov
@ 2017-11-04 10:32 ` Jakub Kicinski
0 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2017-11-04 10:32 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: netdev, oss-drivers, daniel
On Sat, 4 Nov 2017 18:45:31 +0900, Alexei Starovoitov wrote:
> On Fri, Nov 03, 2017 at 01:56:18PM -0700, Jakub Kicinski wrote:
> > Extend struct bpf_prog_info to contain information about program
> > being bound to a device. Since the netdev may get destroyed while
> > program still exists we need a flag to indicate the program is
> > loaded for a device, even if the device is gone.
> >
> > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > Reviewed-by: Simon Horman <simon.horman@netronome.com>
> > Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
> > ---
> > include/linux/bpf.h | 1 +
> > include/uapi/linux/bpf.h | 6 ++++++
> > kernel/bpf/offload.c | 12 ++++++++++++
> > kernel/bpf/syscall.c | 5 +++++
> > 4 files changed, 24 insertions(+)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index e45d43f9ec92..98bacd0fa5cc 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -506,6 +506,7 @@ static inline int cpu_map_enqueue(struct bpf_cpu_map_entry *rcpu,
> >
> > int bpf_prog_offload_compile(struct bpf_prog *prog);
> > void bpf_prog_offload_destroy(struct bpf_prog *prog);
> > +u32 bpf_prog_offload_ifindex(struct bpf_prog *prog);
> >
> > #if defined(CONFIG_NET) && defined(CONFIG_BPF_SYSCALL)
> > int bpf_prog_offload_init(struct bpf_prog *prog, union bpf_attr *attr);
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 727a3dba13e6..e92f62cf933a 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -894,6 +894,10 @@ enum sk_action {
> >
> > #define BPF_TAG_SIZE 8
> >
> > +enum bpf_prog_status {
> > + BPF_PROG_STATUS_DEV_BOUND = (1 << 0),
> > +};
> > +
> > struct bpf_prog_info {
> > __u32 type;
> > __u32 id;
> > @@ -907,6 +911,8 @@ struct bpf_prog_info {
> > __u32 nr_map_ids;
> > __aligned_u64 map_ids;
> > char name[BPF_OBJ_NAME_LEN];
> > + __u32 ifindex;
> > + __u32 status;
>
> why status is needed?
> ifindex cannot be zero, so if it's set > 0 would mean
> that the program is bound.
Devices may come and go, independently from the lifetime of the program,
therefore there is a notion of a program which has been loaded for a
particular device but the device is gone (and therefore its ifindex is
meaningless). I tried to explain this in the commit message.
> Also would be good to have consistent name with prog_load.
> imo prog_target_ifindex is too long.
> May be call it 'ifindex' both in bpf_attr and in bpf_prog_info ?
Perhaps I'm missing something, but bpf_attr is a huge union of (mostly)
unnamed anonymous structs. I foresee that we will have to add an
ifindex member for a map command as well, therefore the prog_* prefix
seems prudent. Should I go back to prog_ifindex in bpf_attr?
Or perhaps should I duplicate the struct for BPF_PROG_LOAD but this
time give it a member name so we can extend it without worrying about
member name conflicts?
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH net-next v2 04/15] bpftool: print program device bound info
2017-11-03 20:56 [PATCH net-next v2 00/15] bpf: add offload as a first class citizen Jakub Kicinski
` (2 preceding siblings ...)
2017-11-03 20:56 ` [PATCH net-next v2 03/15] bpf: report offload info to user space Jakub Kicinski
@ 2017-11-03 20:56 ` Jakub Kicinski
2017-11-03 20:56 ` [PATCH net-next v2 05/15] xdp: allow attaching programs loaded for specific device Jakub Kicinski
` (11 subsequent siblings)
15 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2017-11-03 20:56 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, Jakub Kicinski
If program is bound to a device, print the name of the relevant
interface or unknown if the netdev has since been removed.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
tools/bpf/bpftool/prog.c | 31 +++++++++++++++++++++++++++++++
tools/include/uapi/linux/bpf.h | 7 +++++++
2 files changed, 38 insertions(+)
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 250f80fd46aa..d3ab808dc882 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -41,6 +41,7 @@
#include <string.h>
#include <time.h>
#include <unistd.h>
+#include <net/if.h>
#include <sys/types.h>
#include <sys/stat.h>
@@ -229,6 +230,21 @@ static void print_prog_json(struct bpf_prog_info *info, int fd)
info->tag[0], info->tag[1], info->tag[2], info->tag[3],
info->tag[4], info->tag[5], info->tag[6], info->tag[7]);
+ if (info->status & BPF_PROG_STATUS_DEV_BOUND) {
+ jsonw_name(json_wtr, "dev");
+ if (info->ifindex) {
+ char name[IF_NAMESIZE];
+
+ if (!if_indextoname(info->ifindex, name))
+ jsonw_printf(json_wtr, "\"ifindex:%d\"",
+ info->ifindex);
+ else
+ jsonw_printf(json_wtr, "\"%s\"", name);
+ } else {
+ jsonw_printf(json_wtr, "\"unknown\"");
+ }
+ }
+
if (info->load_time) {
char buf[32];
@@ -274,6 +290,21 @@ static void print_prog_plain(struct bpf_prog_info *info, int fd)
printf("tag ");
fprint_hex(stdout, info->tag, BPF_TAG_SIZE, "");
+ printf(" ");
+
+ if (info->status & BPF_PROG_STATUS_DEV_BOUND) {
+ printf("dev ");
+ if (info->ifindex) {
+ char name[IF_NAMESIZE];
+
+ if (!if_indextoname(info->ifindex, name))
+ printf("ifindex:%d ", info->ifindex);
+ else
+ printf("%s ", name);
+ } else {
+ printf("unknown ");
+ }
+ }
printf("\n");
if (info->load_time) {
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 7cebba491011..e92f62cf933a 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -259,6 +259,7 @@ union bpf_attr {
__u32 kern_version; /* checked when prog_type=kprobe */
__u32 prog_flags;
char prog_name[BPF_OBJ_NAME_LEN];
+ __u32 prog_target_ifindex; /* ifindex of netdev to prep for */
};
struct { /* anonymous struct used by BPF_OBJ_* commands */
@@ -893,6 +894,10 @@ enum sk_action {
#define BPF_TAG_SIZE 8
+enum bpf_prog_status {
+ BPF_PROG_STATUS_DEV_BOUND = (1 << 0),
+};
+
struct bpf_prog_info {
__u32 type;
__u32 id;
@@ -906,6 +911,8 @@ struct bpf_prog_info {
__u32 nr_map_ids;
__aligned_u64 map_ids;
char name[BPF_OBJ_NAME_LEN];
+ __u32 ifindex;
+ __u32 status;
} __attribute__((aligned(8)));
struct bpf_map_info {
--
2.14.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH net-next v2 05/15] xdp: allow attaching programs loaded for specific device
2017-11-03 20:56 [PATCH net-next v2 00/15] bpf: add offload as a first class citizen Jakub Kicinski
` (3 preceding siblings ...)
2017-11-03 20:56 ` [PATCH net-next v2 04/15] bpftool: print program device bound info Jakub Kicinski
@ 2017-11-03 20:56 ` Jakub Kicinski
2017-11-12 9:00 ` Jiri Pirko
2017-11-03 20:56 ` [PATCH net-next v2 06/15] cls_bpf: " Jakub Kicinski
` (10 subsequent siblings)
15 siblings, 1 reply; 23+ messages in thread
From: Jakub Kicinski @ 2017-11-03 20:56 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, Jakub Kicinski
Pass the netdev pointer to bpf_prog_get_type(). This way
BPF code can decide whether the device matches what the
code was loaded/translated for.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
include/linux/bpf.h | 10 ++++++++++
kernel/bpf/syscall.c | 33 +++++++++++++++++++++++++++++----
net/core/dev.c | 6 +++++-
3 files changed, 44 insertions(+), 5 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 98bacd0fa5cc..c397934f91dd 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -335,6 +335,8 @@ extern const struct bpf_verifier_ops xdp_analyzer_ops;
struct bpf_prog *bpf_prog_get(u32 ufd);
struct bpf_prog *bpf_prog_get_type(u32 ufd, enum bpf_prog_type type);
+struct bpf_prog *bpf_prog_get_type_dev(u32 ufd, enum bpf_prog_type type,
+ struct net_device *netdev);
struct bpf_prog * __must_check bpf_prog_add(struct bpf_prog *prog, int i);
void bpf_prog_sub(struct bpf_prog *prog, int i);
struct bpf_prog * __must_check bpf_prog_inc(struct bpf_prog *prog);
@@ -428,6 +430,14 @@ static inline struct bpf_prog *bpf_prog_get_type(u32 ufd,
{
return ERR_PTR(-EOPNOTSUPP);
}
+
+static inline struct bpf_prog *bpf_prog_get_type_dev(u32 ufd,
+ enum bpf_prog_type type,
+ struct net_device *netdev)
+{
+ return ERR_PTR(-EOPNOTSUPP);
+}
+
static inline struct bpf_prog * __must_check bpf_prog_add(struct bpf_prog *prog,
int i)
{
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 3217c20ea91b..68f9123acd39 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1057,7 +1057,22 @@ struct bpf_prog *bpf_prog_inc_not_zero(struct bpf_prog *prog)
}
EXPORT_SYMBOL_GPL(bpf_prog_inc_not_zero);
-static struct bpf_prog *__bpf_prog_get(u32 ufd, enum bpf_prog_type *attach_type)
+static bool bpf_prog_can_attach(struct bpf_prog *prog,
+ enum bpf_prog_type *attach_type,
+ struct net_device *netdev)
+{
+ struct bpf_dev_offload *offload = prog->aux->offload;
+
+ if (prog->type != *attach_type)
+ return false;
+ if (offload && offload->netdev != netdev)
+ return false;
+
+ return true;
+}
+
+static struct bpf_prog *__bpf_prog_get(u32 ufd, enum bpf_prog_type *attach_type,
+ struct net_device *netdev)
{
struct fd f = fdget(ufd);
struct bpf_prog *prog;
@@ -1065,7 +1080,7 @@ static struct bpf_prog *__bpf_prog_get(u32 ufd, enum bpf_prog_type *attach_type)
prog = ____bpf_prog_get(f);
if (IS_ERR(prog))
return prog;
- if (attach_type && (prog->type != *attach_type || prog->aux->offload)) {
+ if (attach_type && !bpf_prog_can_attach(prog, attach_type, netdev)) {
prog = ERR_PTR(-EINVAL);
goto out;
}
@@ -1078,12 +1093,12 @@ static struct bpf_prog *__bpf_prog_get(u32 ufd, enum bpf_prog_type *attach_type)
struct bpf_prog *bpf_prog_get(u32 ufd)
{
- return __bpf_prog_get(ufd, NULL);
+ return __bpf_prog_get(ufd, NULL, NULL);
}
struct bpf_prog *bpf_prog_get_type(u32 ufd, enum bpf_prog_type type)
{
- struct bpf_prog *prog = __bpf_prog_get(ufd, &type);
+ struct bpf_prog *prog = __bpf_prog_get(ufd, &type, NULL);
if (!IS_ERR(prog))
trace_bpf_prog_get_type(prog);
@@ -1091,6 +1106,16 @@ struct bpf_prog *bpf_prog_get_type(u32 ufd, enum bpf_prog_type type)
}
EXPORT_SYMBOL_GPL(bpf_prog_get_type);
+struct bpf_prog *bpf_prog_get_type_dev(u32 ufd, enum bpf_prog_type type,
+ struct net_device *netdev)
+{
+ struct bpf_prog *prog = __bpf_prog_get(ufd, &type, netdev);
+
+ if (!IS_ERR(prog))
+ trace_bpf_prog_get_type(prog);
+ return prog;
+}
+
/* last field in 'union bpf_attr' used by this command */
#define BPF_PROG_LOAD_LAST_FIELD prog_target_ifindex
diff --git a/net/core/dev.c b/net/core/dev.c
index 10cde58d3275..30b5fe32c525 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7157,7 +7157,11 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
__dev_xdp_attached(dev, bpf_op, NULL))
return -EBUSY;
- prog = bpf_prog_get_type(fd, BPF_PROG_TYPE_XDP);
+ if (bpf_op == ops->ndo_bpf)
+ prog = bpf_prog_get_type_dev(fd, BPF_PROG_TYPE_XDP,
+ dev);
+ else
+ prog = bpf_prog_get_type(fd, BPF_PROG_TYPE_XDP);
if (IS_ERR(prog))
return PTR_ERR(prog);
}
--
2.14.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH net-next v2 05/15] xdp: allow attaching programs loaded for specific device
2017-11-03 20:56 ` [PATCH net-next v2 05/15] xdp: allow attaching programs loaded for specific device Jakub Kicinski
@ 2017-11-12 9:00 ` Jiri Pirko
2017-11-12 19:33 ` Daniel Borkmann
0 siblings, 1 reply; 23+ messages in thread
From: Jiri Pirko @ 2017-11-12 9:00 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, oss-drivers, alexei.starovoitov, daniel
Fri, Nov 03, 2017 at 09:56:20PM CET, jakub.kicinski@netronome.com wrote:
>Pass the netdev pointer to bpf_prog_get_type(). This way
>BPF code can decide whether the device matches what the
>code was loaded/translated for.
>
[...]
>diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>index 3217c20ea91b..68f9123acd39 100644
>--- a/kernel/bpf/syscall.c
>+++ b/kernel/bpf/syscall.c
>@@ -1057,7 +1057,22 @@ struct bpf_prog *bpf_prog_inc_not_zero(struct bpf_prog *prog)
> }
> EXPORT_SYMBOL_GPL(bpf_prog_inc_not_zero);
>
>-static struct bpf_prog *__bpf_prog_get(u32 ufd, enum bpf_prog_type *attach_type)
>+static bool bpf_prog_can_attach(struct bpf_prog *prog,
>+ enum bpf_prog_type *attach_type,
>+ struct net_device *netdev)
>+{
>+ struct bpf_dev_offload *offload = prog->aux->offload;
>+
>+ if (prog->type != *attach_type)
>+ return false;
>+ if (offload && offload->netdev != netdev)
This means you return false in case netdev function arg is NULL. Is that
intentional?
Seems wrong to me because for example in cls_bpf_prog_from_efd, you
would get an error in case TCA_CLS_FLAGS_SKIP_SW is not set.
>+ return false;
>+
>+ return true;
>+}
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH net-next v2 05/15] xdp: allow attaching programs loaded for specific device
2017-11-12 9:00 ` Jiri Pirko
@ 2017-11-12 19:33 ` Daniel Borkmann
0 siblings, 0 replies; 23+ messages in thread
From: Daniel Borkmann @ 2017-11-12 19:33 UTC (permalink / raw)
To: Jiri Pirko, Jakub Kicinski; +Cc: netdev, oss-drivers, alexei.starovoitov
On 11/12/2017 10:00 AM, Jiri Pirko wrote:
> Fri, Nov 03, 2017 at 09:56:20PM CET, jakub.kicinski@netronome.com wrote:
>> Pass the netdev pointer to bpf_prog_get_type(). This way
>> BPF code can decide whether the device matches what the
>> code was loaded/translated for.
>
> [...]
>
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index 3217c20ea91b..68f9123acd39 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -1057,7 +1057,22 @@ struct bpf_prog *bpf_prog_inc_not_zero(struct bpf_prog *prog)
>> }
>> EXPORT_SYMBOL_GPL(bpf_prog_inc_not_zero);
>>
>> -static struct bpf_prog *__bpf_prog_get(u32 ufd, enum bpf_prog_type *attach_type)
>> +static bool bpf_prog_can_attach(struct bpf_prog *prog,
>> + enum bpf_prog_type *attach_type,
>> + struct net_device *netdev)
>> +{
>> + struct bpf_dev_offload *offload = prog->aux->offload;
>> +
>> + if (prog->type != *attach_type)
>> + return false;
>> + if (offload && offload->netdev != netdev)
>
> This means you return false in case netdev function arg is NULL. Is that
> intentional?
>
> Seems wrong to me because for example in cls_bpf_prog_from_efd, you
> would get an error in case TCA_CLS_FLAGS_SKIP_SW is not set.
I think it's fine, the prog was originally loaded in the verifier pass
to be JITed via nfp JIT, so you'll have a valid prog->aux->offload, and
you're specifying TCA_CLS_FLAGS_SKIP_SW for fetching the qdisc dev where
we match devs above. So if that dev is not set (NULL) e.g. due to intention
of running the specified prog in sw, then you cannot use that bpf_prog
which was loaded as offloaded one instead. Looks correct to me.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH net-next v2 06/15] cls_bpf: allow attaching programs loaded for specific device
2017-11-03 20:56 [PATCH net-next v2 00/15] bpf: add offload as a first class citizen Jakub Kicinski
` (4 preceding siblings ...)
2017-11-03 20:56 ` [PATCH net-next v2 05/15] xdp: allow attaching programs loaded for specific device Jakub Kicinski
@ 2017-11-03 20:56 ` Jakub Kicinski
2017-11-03 20:56 ` [PATCH net-next v2 07/15] nfp: bpf: drop support for cls_bpf with legacy actions Jakub Kicinski
` (9 subsequent siblings)
15 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2017-11-03 20:56 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, Jakub Kicinski
If TC program is loaded with skip_sw flag, we should allow
the device-specific programs to be accepted.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
kernel/bpf/syscall.c | 1 +
net/sched/cls_bpf.c | 10 +++++++---
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 68f9123acd39..416d70cdfc76 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1115,6 +1115,7 @@ struct bpf_prog *bpf_prog_get_type_dev(u32 ufd, enum bpf_prog_type type,
trace_bpf_prog_get_type(prog);
return prog;
}
+EXPORT_SYMBOL_GPL(bpf_prog_get_type_dev);
/* last field in 'union bpf_attr' used by this command */
#define BPF_PROG_LOAD_LAST_FIELD prog_target_ifindex
diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index bc3edde1b9d7..dc9bd9a0070b 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -374,7 +374,7 @@ static int cls_bpf_prog_from_ops(struct nlattr **tb, struct cls_bpf_prog *prog)
}
static int cls_bpf_prog_from_efd(struct nlattr **tb, struct cls_bpf_prog *prog,
- const struct tcf_proto *tp)
+ u32 gen_flags, const struct tcf_proto *tp)
{
struct bpf_prog *fp;
char *name = NULL;
@@ -382,7 +382,11 @@ static int cls_bpf_prog_from_efd(struct nlattr **tb, struct cls_bpf_prog *prog,
bpf_fd = nla_get_u32(tb[TCA_BPF_FD]);
- fp = bpf_prog_get_type(bpf_fd, BPF_PROG_TYPE_SCHED_CLS);
+ if (gen_flags & TCA_CLS_FLAGS_SKIP_SW)
+ fp = bpf_prog_get_type_dev(bpf_fd, BPF_PROG_TYPE_SCHED_CLS,
+ qdisc_dev(tp->q));
+ else
+ fp = bpf_prog_get_type(bpf_fd, BPF_PROG_TYPE_SCHED_CLS);
if (IS_ERR(fp))
return PTR_ERR(fp);
@@ -440,7 +444,7 @@ static int cls_bpf_set_parms(struct net *net, struct tcf_proto *tp,
prog->gen_flags = gen_flags;
ret = is_bpf ? cls_bpf_prog_from_ops(tb, prog) :
- cls_bpf_prog_from_efd(tb, prog, tp);
+ cls_bpf_prog_from_efd(tb, prog, gen_flags, tp);
if (ret < 0)
return ret;
--
2.14.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH net-next v2 07/15] nfp: bpf: drop support for cls_bpf with legacy actions
2017-11-03 20:56 [PATCH net-next v2 00/15] bpf: add offload as a first class citizen Jakub Kicinski
` (5 preceding siblings ...)
2017-11-03 20:56 ` [PATCH net-next v2 06/15] cls_bpf: " Jakub Kicinski
@ 2017-11-03 20:56 ` Jakub Kicinski
2017-11-03 20:56 ` [PATCH net-next v2 08/15] nfp: bpf: remove the register renumbering leftovers Jakub Kicinski
` (8 subsequent siblings)
15 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2017-11-03 20:56 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, Jakub Kicinski
Only support BPF_PROG_TYPE_SCHED_CLS programs in direct
action mode. This simplifies preparing the offload since
there will now be only one mode of operation for that type
of program. We need to know the attachment mode type of
cls_bpf programs, because exit codes are interpreted
differently for legacy vs DA mode.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
drivers/net/ethernet/netronome/nfp/bpf/jit.c | 87 ++---------------
drivers/net/ethernet/netronome/nfp/bpf/main.c | 33 ++-----
drivers/net/ethernet/netronome/nfp/bpf/main.h | 30 +-----
drivers/net/ethernet/netronome/nfp/bpf/offload.c | 108 +---------------------
drivers/net/ethernet/netronome/nfp/bpf/verifier.c | 11 +--
5 files changed, 22 insertions(+), 247 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/jit.c b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
index 2609a2487100..e1907a1d269e 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/jit.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
@@ -201,47 +201,6 @@ emit_br(struct nfp_prog *nfp_prog, enum br_mask mask, u16 addr, u8 defer)
BR_CSS_NONE, addr, defer);
}
-static void
-__emit_br_byte(struct nfp_prog *nfp_prog, u8 areg, u8 breg, bool imm8,
- u8 byte, bool equal, u16 addr, u8 defer, bool src_lmextn)
-{
- u16 addr_lo, addr_hi;
- u64 insn;
-
- addr_lo = addr & (OP_BB_ADDR_LO >> __bf_shf(OP_BB_ADDR_LO));
- addr_hi = addr != addr_lo;
-
- insn = OP_BBYTE_BASE |
- FIELD_PREP(OP_BB_A_SRC, areg) |
- FIELD_PREP(OP_BB_BYTE, byte) |
- FIELD_PREP(OP_BB_B_SRC, breg) |
- FIELD_PREP(OP_BB_I8, imm8) |
- FIELD_PREP(OP_BB_EQ, equal) |
- FIELD_PREP(OP_BB_DEFBR, defer) |
- FIELD_PREP(OP_BB_ADDR_LO, addr_lo) |
- FIELD_PREP(OP_BB_ADDR_HI, addr_hi) |
- FIELD_PREP(OP_BB_SRC_LMEXTN, src_lmextn);
-
- nfp_prog_push(nfp_prog, insn);
-}
-
-static void
-emit_br_byte_neq(struct nfp_prog *nfp_prog,
- swreg src, u8 imm, u8 byte, u16 addr, u8 defer)
-{
- struct nfp_insn_re_regs reg;
- int err;
-
- err = swreg_to_restricted(reg_none(), src, reg_imm(imm), ®, true);
- if (err) {
- nfp_prog->error = err;
- return;
- }
-
- __emit_br_byte(nfp_prog, reg.areg, reg.breg, reg.i8, byte, false, addr,
- defer, reg.src_lmextn);
-}
-
static void
__emit_immed(struct nfp_prog *nfp_prog, u16 areg, u16 breg, u16 imm_hi,
enum immed_width width, bool invert,
@@ -1547,7 +1506,7 @@ mem_ldx(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta,
unsigned int size)
{
if (meta->ptr.type == PTR_TO_CTX) {
- if (nfp_prog->act == NN_ACT_XDP)
+ if (nfp_prog->type == BPF_PROG_TYPE_XDP)
return mem_ldx_xdp(nfp_prog, meta, size);
else
return mem_ldx_skb(nfp_prog, meta, size);
@@ -2022,34 +1981,6 @@ static void nfp_intro(struct nfp_prog *nfp_prog)
plen_reg(nfp_prog), ALU_OP_AND, pv_len(nfp_prog));
}
-static void nfp_outro_tc_legacy(struct nfp_prog *nfp_prog)
-{
- const u8 act2code[] = {
- [NN_ACT_TC_DROP] = 0x22,
- [NN_ACT_TC_REDIR] = 0x24
- };
- /* Target for aborts */
- nfp_prog->tgt_abort = nfp_prog_current_offset(nfp_prog);
- wrp_immed(nfp_prog, reg_both(0), 0);
-
- /* Target for normal exits */
- nfp_prog->tgt_out = nfp_prog_current_offset(nfp_prog);
- /* Legacy TC mode:
- * 0 0x11 -> pass, count as stat0
- * -1 drop 0x22 -> drop, count as stat1
- * redir 0x24 -> redir, count as stat1
- * ife mark 0x21 -> pass, count as stat1
- * ife + tx 0x24 -> redir, count as stat1
- */
- emit_br_byte_neq(nfp_prog, reg_b(0), 0xff, 0, nfp_prog->tgt_done, 2);
- wrp_mov(nfp_prog, reg_a(0), NFP_BPF_ABI_FLAGS);
- emit_ld_field(nfp_prog, reg_a(0), 0xc, reg_imm(0x11), SHF_SC_L_SHF, 16);
-
- emit_br(nfp_prog, BR_UNC, nfp_prog->tgt_done, 1);
- emit_ld_field(nfp_prog, reg_a(0), 0xc, reg_imm(act2code[nfp_prog->act]),
- SHF_SC_L_SHF, 16);
-}
-
static void nfp_outro_tc_da(struct nfp_prog *nfp_prog)
{
/* TC direct-action mode:
@@ -2142,17 +2073,15 @@ static void nfp_outro_xdp(struct nfp_prog *nfp_prog)
static void nfp_outro(struct nfp_prog *nfp_prog)
{
- switch (nfp_prog->act) {
- case NN_ACT_DIRECT:
+ switch (nfp_prog->type) {
+ case BPF_PROG_TYPE_SCHED_CLS:
nfp_outro_tc_da(nfp_prog);
break;
- case NN_ACT_TC_DROP:
- case NN_ACT_TC_REDIR:
- nfp_outro_tc_legacy(nfp_prog);
- break;
- case NN_ACT_XDP:
+ case BPF_PROG_TYPE_XDP:
nfp_outro_xdp(nfp_prog);
break;
+ default:
+ WARN_ON(1);
}
}
@@ -2351,7 +2280,6 @@ static int nfp_bpf_ustore_calc(struct nfp_prog *nfp_prog, __le64 *ustore)
* nfp_bpf_jit() - translate BPF code into NFP assembly
* @filter: kernel BPF filter struct
* @prog_mem: memory to store assembler instructions
- * @act: action attached to this eBPF program
* @prog_start: offset of the first instruction when loaded
* @prog_done: where to jump on exit
* @prog_sz: size of @prog_mem in instructions
@@ -2359,7 +2287,6 @@ static int nfp_bpf_ustore_calc(struct nfp_prog *nfp_prog, __le64 *ustore)
*/
int
nfp_bpf_jit(struct bpf_prog *filter, void *prog_mem,
- enum nfp_bpf_action_type act,
unsigned int prog_start, unsigned int prog_done,
unsigned int prog_sz, struct nfp_bpf_result *res)
{
@@ -2371,7 +2298,7 @@ nfp_bpf_jit(struct bpf_prog *filter, void *prog_mem,
return -ENOMEM;
INIT_LIST_HEAD(&nfp_prog->insns);
- nfp_prog->act = act;
+ nfp_prog->type = filter->type;
nfp_prog->start_off = prog_start;
nfp_prog->tgt_done = prog_done;
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.c b/drivers/net/ethernet/netronome/nfp/bpf/main.c
index 8e3e89cace8d..2ff97f12c160 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.c
@@ -85,34 +85,10 @@ static const char *nfp_bpf_extra_cap(struct nfp_app *app, struct nfp_net *nn)
return nfp_net_ebpf_capable(nn) ? "BPF" : "";
}
-static int
-nfp_bpf_vnic_alloc(struct nfp_app *app, struct nfp_net *nn, unsigned int id)
-{
- struct nfp_net_bpf_priv *priv;
- int ret;
-
- priv = kmalloc(sizeof(*priv), GFP_KERNEL);
- if (!priv)
- return -ENOMEM;
-
- nn->app_priv = priv;
- spin_lock_init(&priv->rx_filter_lock);
- priv->nn = nn;
- timer_setup(&priv->rx_filter_stats_timer,
- nfp_net_filter_stats_timer, 0);
-
- ret = nfp_app_nic_vnic_alloc(app, nn, id);
- if (ret)
- kfree(priv);
-
- return ret;
-}
-
static void nfp_bpf_vnic_free(struct nfp_app *app, struct nfp_net *nn)
{
if (nn->dp.bpf_offload_xdp)
nfp_bpf_xdp_offload(app, nn, NULL);
- kfree(nn->app_priv);
}
static int nfp_bpf_setup_tc_block_cb(enum tc_setup_type type,
@@ -133,6 +109,13 @@ static int nfp_bpf_setup_tc_block_cb(enum tc_setup_type type,
if (nn->dp.bpf_offload_xdp)
return -EBUSY;
+ /* Only support TC direct action */
+ if (!cls_bpf->exts_integrated ||
+ tcf_exts_has_actions(cls_bpf->exts)) {
+ nn_err(nn, "only direct action with no legacy actions supported\n");
+ return -EOPNOTSUPP;
+ }
+
return nfp_net_bpf_offload(nn, cls_bpf);
default:
return -EOPNOTSUPP;
@@ -184,7 +167,7 @@ const struct nfp_app_type app_bpf = {
.extra_cap = nfp_bpf_extra_cap,
- .vnic_alloc = nfp_bpf_vnic_alloc,
+ .vnic_alloc = nfp_app_nic_vnic_alloc,
.vnic_free = nfp_bpf_vnic_free,
.setup_tc = nfp_bpf_setup_tc,
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.h b/drivers/net/ethernet/netronome/nfp/bpf/main.h
index bc604030ff6c..c5280de2ab14 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.h
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.h
@@ -65,13 +65,6 @@ enum pkt_vec {
PKT_VEC_PKT_PTR = 2,
};
-enum nfp_bpf_action_type {
- NN_ACT_TC_DROP,
- NN_ACT_TC_REDIR,
- NN_ACT_DIRECT,
- NN_ACT_XDP,
-};
-
#define pv_len(np) reg_lm(1, PKT_VEC_PKT_LEN)
#define pv_ctm_ptr(np) reg_lm(1, PKT_VEC_PKT_PTR)
@@ -147,7 +140,7 @@ static inline u8 mbpf_mode(const struct nfp_insn_meta *meta)
* @prog: machine code
* @prog_len: number of valid instructions in @prog array
* @__prog_alloc_len: alloc size of @prog array
- * @act: BPF program/action type (TC DA, TC with action, XDP etc.)
+ * @type: BPF program type
* @num_regs: number of registers used by this program
* @regs_per_thread: number of basic registers allocated per thread
* @start_off: address of the first instruction in the memory
@@ -164,7 +157,7 @@ struct nfp_prog {
unsigned int prog_len;
unsigned int __prog_alloc_len;
- enum nfp_bpf_action_type act;
+ enum bpf_prog_type type;
unsigned int num_regs;
unsigned int regs_per_thread;
@@ -188,7 +181,7 @@ struct nfp_bpf_result {
};
int
-nfp_bpf_jit(struct bpf_prog *filter, void *prog, enum nfp_bpf_action_type act,
+nfp_bpf_jit(struct bpf_prog *filter, void *prog,
unsigned int prog_start, unsigned int prog_done,
unsigned int prog_sz, struct nfp_bpf_result *res);
@@ -197,23 +190,6 @@ int nfp_prog_verify(struct nfp_prog *nfp_prog, struct bpf_prog *prog);
struct nfp_net;
struct tc_cls_bpf_offload;
-/**
- * struct nfp_net_bpf_priv - per-vNIC BPF private data
- * @rx_filter: Filter offload statistics - dropped packets/bytes
- * @rx_filter_prev: Filter offload statistics - values from previous update
- * @rx_filter_change: Jiffies when statistics last changed
- * @rx_filter_stats_timer: Timer for polling filter offload statistics
- * @rx_filter_lock: Lock protecting timer state changes (teardown)
- */
-struct nfp_net_bpf_priv {
- struct nfp_stat_pair rx_filter, rx_filter_prev;
- unsigned long rx_filter_change;
- struct timer_list rx_filter_stats_timer;
- struct nfp_net *nn;
- spinlock_t rx_filter_lock;
-};
-
int nfp_net_bpf_offload(struct nfp_net *nn, struct tc_cls_bpf_offload *cls_bpf);
-void nfp_net_filter_stats_timer(struct timer_list *t);
#endif
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
index 6d576f631392..b9b5d675c4d3 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
@@ -51,92 +51,6 @@
#include "../nfp_net_ctrl.h"
#include "../nfp_net.h"
-void nfp_net_filter_stats_timer(struct timer_list *t)
-{
- struct nfp_net_bpf_priv *priv = from_timer(priv, t,
- rx_filter_stats_timer);
- struct nfp_net *nn = priv->nn;
- struct nfp_stat_pair latest;
-
- spin_lock_bh(&priv->rx_filter_lock);
-
- if (nn->dp.ctrl & NFP_NET_CFG_CTRL_BPF)
- mod_timer(&priv->rx_filter_stats_timer,
- jiffies + NFP_NET_STAT_POLL_IVL);
-
- spin_unlock_bh(&priv->rx_filter_lock);
-
- latest.pkts = nn_readq(nn, NFP_NET_CFG_STATS_APP1_FRAMES);
- latest.bytes = nn_readq(nn, NFP_NET_CFG_STATS_APP1_BYTES);
-
- if (latest.pkts != priv->rx_filter.pkts)
- priv->rx_filter_change = jiffies;
-
- priv->rx_filter = latest;
-}
-
-static void nfp_net_bpf_stats_reset(struct nfp_net *nn)
-{
- struct nfp_net_bpf_priv *priv = nn->app_priv;
-
- priv->rx_filter.pkts = nn_readq(nn, NFP_NET_CFG_STATS_APP1_FRAMES);
- priv->rx_filter.bytes = nn_readq(nn, NFP_NET_CFG_STATS_APP1_BYTES);
- priv->rx_filter_prev = priv->rx_filter;
- priv->rx_filter_change = jiffies;
-}
-
-static int
-nfp_net_bpf_stats_update(struct nfp_net *nn, struct tc_cls_bpf_offload *cls_bpf)
-{
- struct nfp_net_bpf_priv *priv = nn->app_priv;
- u64 bytes, pkts;
-
- pkts = priv->rx_filter.pkts - priv->rx_filter_prev.pkts;
- bytes = priv->rx_filter.bytes - priv->rx_filter_prev.bytes;
- bytes -= pkts * ETH_HLEN;
-
- priv->rx_filter_prev = priv->rx_filter;
-
- tcf_exts_stats_update(cls_bpf->exts,
- bytes, pkts, priv->rx_filter_change);
-
- return 0;
-}
-
-static int
-nfp_net_bpf_get_act(struct nfp_net *nn, struct tc_cls_bpf_offload *cls_bpf)
-{
- const struct tc_action *a;
- LIST_HEAD(actions);
-
- if (!cls_bpf->exts)
- return NN_ACT_XDP;
-
- /* TC direct action */
- if (cls_bpf->exts_integrated) {
- if (!tcf_exts_has_actions(cls_bpf->exts))
- return NN_ACT_DIRECT;
-
- return -EOPNOTSUPP;
- }
-
- /* TC legacy mode */
- if (!tcf_exts_has_one_action(cls_bpf->exts))
- return -EOPNOTSUPP;
-
- tcf_exts_to_list(cls_bpf->exts, &actions);
- list_for_each_entry(a, &actions, list) {
- if (is_tcf_gact_shot(a))
- return NN_ACT_TC_DROP;
-
- if (is_tcf_mirred_egress_redirect(a) &&
- tcf_mirred_ifindex(a) == nn->dp.netdev->ifindex)
- return NN_ACT_TC_REDIR;
- }
-
- return -EOPNOTSUPP;
-}
-
static int
nfp_net_bpf_offload_prepare(struct nfp_net *nn,
struct tc_cls_bpf_offload *cls_bpf,
@@ -144,17 +58,11 @@ nfp_net_bpf_offload_prepare(struct nfp_net *nn,
void **code, dma_addr_t *dma_addr, u16 max_instr)
{
unsigned int code_sz = max_instr * sizeof(u64);
- enum nfp_bpf_action_type act;
unsigned int stack_size;
u16 start_off, done_off;
unsigned int max_mtu;
int ret;
- ret = nfp_net_bpf_get_act(nn, cls_bpf);
- if (ret < 0)
- return ret;
- act = ret;
-
max_mtu = nn_readb(nn, NFP_NET_CFG_BPF_INL_MTU) * 64 - 32;
if (max_mtu < nn->dp.netdev->mtu) {
nn_info(nn, "BPF offload not supported with MTU larger than HW packet split boundary\n");
@@ -175,7 +83,7 @@ nfp_net_bpf_offload_prepare(struct nfp_net *nn,
if (!*code)
return -ENOMEM;
- ret = nfp_bpf_jit(cls_bpf->prog, *code, act, start_off, done_off,
+ ret = nfp_bpf_jit(cls_bpf->prog, *code, start_off, done_off,
max_instr, res);
if (ret)
goto out;
@@ -193,7 +101,6 @@ nfp_net_bpf_load_and_start(struct nfp_net *nn, u32 tc_flags,
unsigned int code_sz, unsigned int n_instr,
bool dense_mode)
{
- struct nfp_net_bpf_priv *priv = nn->app_priv;
u64 bpf_addr = dma_addr;
int err;
@@ -218,25 +125,15 @@ nfp_net_bpf_load_and_start(struct nfp_net *nn, u32 tc_flags,
nn_err(nn, "FW command error while enabling BPF: %d\n", err);
dma_free_coherent(nn->dp.dev, code_sz, code, dma_addr);
-
- nfp_net_bpf_stats_reset(nn);
- mod_timer(&priv->rx_filter_stats_timer,
- jiffies + NFP_NET_STAT_POLL_IVL);
}
static int nfp_net_bpf_stop(struct nfp_net *nn)
{
- struct nfp_net_bpf_priv *priv = nn->app_priv;
-
if (!(nn->dp.ctrl & NFP_NET_CFG_CTRL_BPF))
return 0;
- spin_lock_bh(&priv->rx_filter_lock);
nn->dp.ctrl &= ~NFP_NET_CFG_CTRL_BPF;
- spin_unlock_bh(&priv->rx_filter_lock);
nn_writel(nn, NFP_NET_CFG_CTRL, nn->dp.ctrl);
-
- del_timer_sync(&priv->rx_filter_stats_timer);
nn->dp.bpf_offload_skip_sw = 0;
return nfp_net_reconfig(nn, NFP_NET_CFG_UPDATE_GEN);
@@ -292,9 +189,6 @@ int nfp_net_bpf_offload(struct nfp_net *nn, struct tc_cls_bpf_offload *cls_bpf)
case TC_CLSBPF_DESTROY:
return nfp_net_bpf_stop(nn);
- case TC_CLSBPF_STATS:
- return nfp_net_bpf_stats_update(nn, cls_bpf);
-
default:
return -EOPNOTSUPP;
}
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/verifier.c b/drivers/net/ethernet/netronome/nfp/bpf/verifier.c
index a8c7615546a9..4f31bdefd331 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/verifier.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/verifier.c
@@ -81,7 +81,7 @@ nfp_bpf_check_exit(struct nfp_prog *nfp_prog,
const struct bpf_reg_state *reg0 = cur_regs(env) + BPF_REG_0;
u64 imm;
- if (nfp_prog->act == NN_ACT_XDP)
+ if (nfp_prog->type == BPF_PROG_TYPE_XDP)
return 0;
if (!(reg0->type == SCALAR_VALUE && tnum_is_const(reg0->var_off))) {
@@ -94,13 +94,8 @@ nfp_bpf_check_exit(struct nfp_prog *nfp_prog,
}
imm = reg0->var_off.value;
- if (nfp_prog->act != NN_ACT_DIRECT && imm != 0 && (imm & ~0U) != ~0U) {
- pr_info("unsupported exit state: %d, imm: %llx\n",
- reg0->type, imm);
- return -EINVAL;
- }
-
- if (nfp_prog->act == NN_ACT_DIRECT && imm <= TC_ACT_REDIRECT &&
+ if (nfp_prog->type == BPF_PROG_TYPE_SCHED_CLS &&
+ imm <= TC_ACT_REDIRECT &&
imm != TC_ACT_SHOT && imm != TC_ACT_STOLEN &&
imm != TC_ACT_QUEUED) {
pr_info("unsupported exit state: %d, imm: %llx\n",
--
2.14.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH net-next v2 08/15] nfp: bpf: remove the register renumbering leftovers
2017-11-03 20:56 [PATCH net-next v2 00/15] bpf: add offload as a first class citizen Jakub Kicinski
` (6 preceding siblings ...)
2017-11-03 20:56 ` [PATCH net-next v2 07/15] nfp: bpf: drop support for cls_bpf with legacy actions Jakub Kicinski
@ 2017-11-03 20:56 ` Jakub Kicinski
2017-11-03 20:56 ` [PATCH net-next v2 09/15] nfp: bpf: remove unnecessary include of nfp_net.h Jakub Kicinski
` (7 subsequent siblings)
15 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2017-11-03 20:56 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, Jakub Kicinski
The register renumbering was removed and will not be coming back
in its old, naive form, given that it would be fundamentally
incompatible with calling functions. Remove the leftovers.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
drivers/net/ethernet/netronome/nfp/bpf/jit.c | 4 ----
drivers/net/ethernet/netronome/nfp/bpf/main.h | 6 ------
drivers/net/ethernet/netronome/nfp/bpf/offload.c | 13 ++++---------
3 files changed, 4 insertions(+), 19 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/jit.c b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
index e1907a1d269e..ff150c27f411 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/jit.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
@@ -2314,9 +2314,6 @@ nfp_bpf_jit(struct bpf_prog *filter, void *prog_mem,
if (ret)
goto out;
- nfp_prog->num_regs = MAX_BPF_REG;
- nfp_prog->regs_per_thread = 32;
-
nfp_prog->prog = prog_mem;
nfp_prog->__prog_alloc_len = prog_sz;
@@ -2331,7 +2328,6 @@ nfp_bpf_jit(struct bpf_prog *filter, void *prog_mem,
ret = nfp_bpf_ustore_calc(nfp_prog, (__force __le64 *)prog_mem);
res->n_instr = nfp_prog->prog_len;
- res->dense_mode = false;
out:
nfp_prog_free(nfp_prog);
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.h b/drivers/net/ethernet/netronome/nfp/bpf/main.h
index c5280de2ab14..85b7d9398cda 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.h
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.h
@@ -141,8 +141,6 @@ static inline u8 mbpf_mode(const struct nfp_insn_meta *meta)
* @prog_len: number of valid instructions in @prog array
* @__prog_alloc_len: alloc size of @prog array
* @type: BPF program type
- * @num_regs: number of registers used by this program
- * @regs_per_thread: number of basic registers allocated per thread
* @start_off: address of the first instruction in the memory
* @tgt_out: jump target for normal exit
* @tgt_abort: jump target for abort (e.g. access outside of packet buffer)
@@ -159,9 +157,6 @@ struct nfp_prog {
enum bpf_prog_type type;
- unsigned int num_regs;
- unsigned int regs_per_thread;
-
unsigned int start_off;
unsigned int tgt_out;
unsigned int tgt_abort;
@@ -177,7 +172,6 @@ struct nfp_prog {
struct nfp_bpf_result {
unsigned int n_instr;
- bool dense_mode;
};
int
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
index b9b5d675c4d3..268ba1ba82db 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
@@ -98,19 +98,14 @@ nfp_net_bpf_offload_prepare(struct nfp_net *nn,
static void
nfp_net_bpf_load_and_start(struct nfp_net *nn, u32 tc_flags,
void *code, dma_addr_t dma_addr,
- unsigned int code_sz, unsigned int n_instr,
- bool dense_mode)
+ unsigned int code_sz, unsigned int n_instr)
{
- u64 bpf_addr = dma_addr;
int err;
nn->dp.bpf_offload_skip_sw = !!(tc_flags & TCA_CLS_FLAGS_SKIP_SW);
- if (dense_mode)
- bpf_addr |= NFP_NET_CFG_BPF_CFG_8CTX;
-
nn_writew(nn, NFP_NET_CFG_BPF_SIZE, n_instr);
- nn_writeq(nn, NFP_NET_CFG_BPF_ADDR, bpf_addr);
+ nn_writeq(nn, NFP_NET_CFG_BPF_ADDR, dma_addr);
/* Load up the JITed code */
err = nfp_net_reconfig(nn, NFP_NET_CFG_UPDATE_BPF);
@@ -169,7 +164,7 @@ int nfp_net_bpf_offload(struct nfp_net *nn, struct tc_cls_bpf_offload *cls_bpf)
nfp_net_bpf_stop(nn);
nfp_net_bpf_load_and_start(nn, cls_bpf->gen_flags, code,
dma_addr, max_instr * sizeof(u64),
- res.n_instr, res.dense_mode);
+ res.n_instr);
return 0;
case TC_CLSBPF_ADD:
@@ -183,7 +178,7 @@ int nfp_net_bpf_offload(struct nfp_net *nn, struct tc_cls_bpf_offload *cls_bpf)
nfp_net_bpf_load_and_start(nn, cls_bpf->gen_flags, code,
dma_addr, max_instr * sizeof(u64),
- res.n_instr, res.dense_mode);
+ res.n_instr);
return 0;
case TC_CLSBPF_DESTROY:
--
2.14.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH net-next v2 09/15] nfp: bpf: remove unnecessary include of nfp_net.h
2017-11-03 20:56 [PATCH net-next v2 00/15] bpf: add offload as a first class citizen Jakub Kicinski
` (7 preceding siblings ...)
2017-11-03 20:56 ` [PATCH net-next v2 08/15] nfp: bpf: remove the register renumbering leftovers Jakub Kicinski
@ 2017-11-03 20:56 ` Jakub Kicinski
2017-11-03 20:56 ` [PATCH net-next v2 10/15] nfp: bpf: refactor offload logic Jakub Kicinski
` (6 subsequent siblings)
15 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2017-11-03 20:56 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, Jakub Kicinski
BPF offload's main header does not need to include nfp_net.h.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
drivers/net/ethernet/netronome/nfp/bpf/main.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.h b/drivers/net/ethernet/netronome/nfp/bpf/main.h
index 85b7d9398cda..9f0df6a9786d 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.h
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.h
@@ -41,7 +41,6 @@
#include <linux/types.h>
#include "../nfp_asm.h"
-#include "../nfp_net.h"
/* For branch fixup logic use up-most byte of branch instruction as scratch
* area. Remember to clear this before sending instructions to HW!
--
2.14.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH net-next v2 10/15] nfp: bpf: refactor offload logic
2017-11-03 20:56 [PATCH net-next v2 00/15] bpf: add offload as a first class citizen Jakub Kicinski
` (8 preceding siblings ...)
2017-11-03 20:56 ` [PATCH net-next v2 09/15] nfp: bpf: remove unnecessary include of nfp_net.h Jakub Kicinski
@ 2017-11-03 20:56 ` Jakub Kicinski
2017-11-03 20:56 ` [PATCH net-next v2 11/15] nfp: bpf: require seamless reload for program replace Jakub Kicinski
` (5 subsequent siblings)
15 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2017-11-03 20:56 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, Jakub Kicinski
We currently create a fake cls_bpf offload object when we want
to offload XDP. Simplify and clarify the code by moving the
TC/XDP specific logic out of common offload code. This is easy
now that we don't support legacy TC actions. We only need the
bpf program and state of the skip_sw flag.
Temporarily set @code to NULL in nfp_net_bpf_offload(), compilers
seem to have trouble recognizing it's always initialized. Next
patches will eliminate that variable.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
drivers/net/ethernet/netronome/nfp/bpf/main.c | 67 +++++++++++-----------
drivers/net/ethernet/netronome/nfp/bpf/main.h | 4 +-
drivers/net/ethernet/netronome/nfp/bpf/offload.c | 73 ++++++++++--------------
3 files changed, 67 insertions(+), 77 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.c b/drivers/net/ethernet/netronome/nfp/bpf/main.c
index 2ff97f12c160..9e1286346d42 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.c
@@ -54,28 +54,25 @@ static int
nfp_bpf_xdp_offload(struct nfp_app *app, struct nfp_net *nn,
struct bpf_prog *prog)
{
- struct tc_cls_bpf_offload cmd = {
- .prog = prog,
- };
+ bool running, xdp_running;
int ret;
if (!nfp_net_ebpf_capable(nn))
return -EINVAL;
- if (nn->dp.ctrl & NFP_NET_CFG_CTRL_BPF) {
- if (!nn->dp.bpf_offload_xdp)
- return prog ? -EBUSY : 0;
- cmd.command = prog ? TC_CLSBPF_REPLACE : TC_CLSBPF_DESTROY;
- } else {
- if (!prog)
- return 0;
- cmd.command = TC_CLSBPF_ADD;
- }
+ running = nn->dp.ctrl & NFP_NET_CFG_CTRL_BPF;
+ xdp_running = running && nn->dp.bpf_offload_xdp;
+
+ if (!prog && !xdp_running)
+ return 0;
+ if (prog && running && !xdp_running)
+ return -EBUSY;
- ret = nfp_net_bpf_offload(nn, &cmd);
+ ret = nfp_net_bpf_offload(nn, prog, running, true);
/* Stop offload if replace not possible */
- if (ret && cmd.command == TC_CLSBPF_REPLACE)
+ if (ret && prog)
nfp_bpf_xdp_offload(app, nn, NULL);
+
nn->dp.bpf_offload_xdp = prog && !ret;
return ret;
}
@@ -96,27 +93,33 @@ static int nfp_bpf_setup_tc_block_cb(enum tc_setup_type type,
{
struct tc_cls_bpf_offload *cls_bpf = type_data;
struct nfp_net *nn = cb_priv;
+ bool skip_sw;
+
+ if (type != TC_SETUP_CLSBPF ||
+ !tc_can_offload(nn->dp.netdev) ||
+ !nfp_net_ebpf_capable(nn) ||
+ cls_bpf->common.protocol != htons(ETH_P_ALL) ||
+ cls_bpf->common.chain_index)
+ return -EOPNOTSUPP;
+ if (nn->dp.bpf_offload_xdp)
+ return -EBUSY;
- if (!tc_can_offload(nn->dp.netdev))
+ /* Only support TC direct action */
+ if (!cls_bpf->exts_integrated ||
+ tcf_exts_has_actions(cls_bpf->exts)) {
+ nn_err(nn, "only direct action with no legacy actions supported\n");
return -EOPNOTSUPP;
+ }
- switch (type) {
- case TC_SETUP_CLSBPF:
- if (!nfp_net_ebpf_capable(nn) ||
- cls_bpf->common.protocol != htons(ETH_P_ALL) ||
- cls_bpf->common.chain_index)
- return -EOPNOTSUPP;
- if (nn->dp.bpf_offload_xdp)
- return -EBUSY;
-
- /* Only support TC direct action */
- if (!cls_bpf->exts_integrated ||
- tcf_exts_has_actions(cls_bpf->exts)) {
- nn_err(nn, "only direct action with no legacy actions supported\n");
- return -EOPNOTSUPP;
- }
-
- return nfp_net_bpf_offload(nn, cls_bpf);
+ skip_sw = !!(cls_bpf->gen_flags & TCA_CLS_FLAGS_SKIP_SW);
+
+ switch (cls_bpf->command) {
+ case TC_CLSBPF_REPLACE:
+ return nfp_net_bpf_offload(nn, cls_bpf->prog, true, !skip_sw);
+ case TC_CLSBPF_ADD:
+ return nfp_net_bpf_offload(nn, cls_bpf->prog, false, !skip_sw);
+ case TC_CLSBPF_DESTROY:
+ return nfp_net_bpf_offload(nn, NULL, true, !skip_sw);
default:
return -EOPNOTSUPP;
}
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.h b/drivers/net/ethernet/netronome/nfp/bpf/main.h
index 9f0df6a9786d..6dddab95d57a 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.h
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.h
@@ -181,8 +181,8 @@ nfp_bpf_jit(struct bpf_prog *filter, void *prog,
int nfp_prog_verify(struct nfp_prog *nfp_prog, struct bpf_prog *prog);
struct nfp_net;
-struct tc_cls_bpf_offload;
-int nfp_net_bpf_offload(struct nfp_net *nn, struct tc_cls_bpf_offload *cls_bpf);
+int nfp_net_bpf_offload(struct nfp_net *nn, struct bpf_prog *prog,
+ bool old_prog, bool sw_fallback);
#endif
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
index 268ba1ba82db..c09efa1a9649 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
@@ -52,8 +52,7 @@
#include "../nfp_net.h"
static int
-nfp_net_bpf_offload_prepare(struct nfp_net *nn,
- struct tc_cls_bpf_offload *cls_bpf,
+nfp_net_bpf_offload_prepare(struct nfp_net *nn, struct bpf_prog *prog,
struct nfp_bpf_result *res,
void **code, dma_addr_t *dma_addr, u16 max_instr)
{
@@ -73,9 +72,9 @@ nfp_net_bpf_offload_prepare(struct nfp_net *nn,
done_off = nn_readw(nn, NFP_NET_CFG_BPF_DONE);
stack_size = nn_readb(nn, NFP_NET_CFG_BPF_STACK_SZ) * 64;
- if (cls_bpf->prog->aux->stack_depth > stack_size) {
+ if (prog->aux->stack_depth > stack_size) {
nn_info(nn, "stack too large: program %dB > FW stack %dB\n",
- cls_bpf->prog->aux->stack_depth, stack_size);
+ prog->aux->stack_depth, stack_size);
return -EOPNOTSUPP;
}
@@ -83,8 +82,7 @@ nfp_net_bpf_offload_prepare(struct nfp_net *nn,
if (!*code)
return -ENOMEM;
- ret = nfp_bpf_jit(cls_bpf->prog, *code, start_off, done_off,
- max_instr, res);
+ ret = nfp_bpf_jit(prog, *code, start_off, done_off, max_instr, res);
if (ret)
goto out;
@@ -96,13 +94,13 @@ nfp_net_bpf_offload_prepare(struct nfp_net *nn,
}
static void
-nfp_net_bpf_load_and_start(struct nfp_net *nn, u32 tc_flags,
+nfp_net_bpf_load_and_start(struct nfp_net *nn, bool sw_fallback,
void *code, dma_addr_t dma_addr,
unsigned int code_sz, unsigned int n_instr)
{
int err;
- nn->dp.bpf_offload_skip_sw = !!(tc_flags & TCA_CLS_FLAGS_SKIP_SW);
+ nn->dp.bpf_offload_skip_sw = !sw_fallback;
nn_writew(nn, NFP_NET_CFG_BPF_SIZE, n_instr);
nn_writeq(nn, NFP_NET_CFG_BPF_ADDR, dma_addr);
@@ -134,7 +132,8 @@ static int nfp_net_bpf_stop(struct nfp_net *nn)
return nfp_net_reconfig(nn, NFP_NET_CFG_UPDATE_GEN);
}
-int nfp_net_bpf_offload(struct nfp_net *nn, struct tc_cls_bpf_offload *cls_bpf)
+int nfp_net_bpf_offload(struct nfp_net *nn, struct bpf_prog *prog,
+ bool old_prog, bool sw_fallback)
{
struct nfp_bpf_result res;
dma_addr_t dma_addr;
@@ -142,49 +141,37 @@ int nfp_net_bpf_offload(struct nfp_net *nn, struct tc_cls_bpf_offload *cls_bpf)
void *code;
int err;
+ /* There is nothing stopping us from implementing seamless
+ * replace but the simple method of loading I adopted in
+ * the firmware does not handle atomic replace (i.e. we have to
+ * stop the BPF offload and re-enable it). Leaking-in a few
+ * frames which didn't have BPF applied in the hardware should
+ * be fine if software fallback is available, though.
+ */
+ if (prog && old_prog && nn->dp.bpf_offload_skip_sw)
+ return -EBUSY;
+
+ /* Something else is loaded, different program type? */
+ if (!old_prog && nn->dp.ctrl & NFP_NET_CFG_CTRL_BPF)
+ return -EBUSY;
+
max_instr = nn_readw(nn, NFP_NET_CFG_BPF_MAX_LEN);
+ code = NULL;
- switch (cls_bpf->command) {
- case TC_CLSBPF_REPLACE:
- /* There is nothing stopping us from implementing seamless
- * replace but the simple method of loading I adopted in
- * the firmware does not handle atomic replace (i.e. we have to
- * stop the BPF offload and re-enable it). Leaking-in a few
- * frames which didn't have BPF applied in the hardware should
- * be fine if software fallback is available, though.
- */
- if (nn->dp.bpf_offload_skip_sw)
- return -EBUSY;
-
- err = nfp_net_bpf_offload_prepare(nn, cls_bpf, &res, &code,
+ if (prog) {
+ err = nfp_net_bpf_offload_prepare(nn, prog, &res, &code,
&dma_addr, max_instr);
if (err)
return err;
+ }
+ if (old_prog)
nfp_net_bpf_stop(nn);
- nfp_net_bpf_load_and_start(nn, cls_bpf->gen_flags, code,
- dma_addr, max_instr * sizeof(u64),
- res.n_instr);
- return 0;
-
- case TC_CLSBPF_ADD:
- if (nn->dp.ctrl & NFP_NET_CFG_CTRL_BPF)
- return -EBUSY;
-
- err = nfp_net_bpf_offload_prepare(nn, cls_bpf, &res, &code,
- &dma_addr, max_instr);
- if (err)
- return err;
- nfp_net_bpf_load_and_start(nn, cls_bpf->gen_flags, code,
+ if (prog)
+ nfp_net_bpf_load_and_start(nn, sw_fallback, code,
dma_addr, max_instr * sizeof(u64),
res.n_instr);
- return 0;
- case TC_CLSBPF_DESTROY:
- return nfp_net_bpf_stop(nn);
-
- default:
- return -EOPNOTSUPP;
- }
+ return 0;
}
--
2.14.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH net-next v2 11/15] nfp: bpf: require seamless reload for program replace
2017-11-03 20:56 [PATCH net-next v2 00/15] bpf: add offload as a first class citizen Jakub Kicinski
` (9 preceding siblings ...)
2017-11-03 20:56 ` [PATCH net-next v2 10/15] nfp: bpf: refactor offload logic Jakub Kicinski
@ 2017-11-03 20:56 ` Jakub Kicinski
2017-11-03 20:56 ` [PATCH net-next v2 12/15] nfp: bpf: move program prepare and free into offload.c Jakub Kicinski
` (4 subsequent siblings)
15 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2017-11-03 20:56 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, Jakub Kicinski
Firmware supports live replacement of programs for quite some
time now. Remove the software-fallback related logic and
depend on the FW for program replace. Seamless reload will
become a requirement if maps are present, anyway.
Load and start stages have to be split now, since replace
only needs a load, start has already been done on add.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
drivers/net/ethernet/netronome/nfp/bpf/main.c | 11 ++---
drivers/net/ethernet/netronome/nfp/bpf/main.h | 2 +-
drivers/net/ethernet/netronome/nfp/bpf/offload.c | 62 ++++++++++++------------
drivers/net/ethernet/netronome/nfp/nfp_net.h | 2 -
4 files changed, 35 insertions(+), 42 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.c b/drivers/net/ethernet/netronome/nfp/bpf/main.c
index 9e1286346d42..7ae7528cd96b 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.c
@@ -68,7 +68,7 @@ nfp_bpf_xdp_offload(struct nfp_app *app, struct nfp_net *nn,
if (prog && running && !xdp_running)
return -EBUSY;
- ret = nfp_net_bpf_offload(nn, prog, running, true);
+ ret = nfp_net_bpf_offload(nn, prog, running);
/* Stop offload if replace not possible */
if (ret && prog)
nfp_bpf_xdp_offload(app, nn, NULL);
@@ -93,7 +93,6 @@ static int nfp_bpf_setup_tc_block_cb(enum tc_setup_type type,
{
struct tc_cls_bpf_offload *cls_bpf = type_data;
struct nfp_net *nn = cb_priv;
- bool skip_sw;
if (type != TC_SETUP_CLSBPF ||
!tc_can_offload(nn->dp.netdev) ||
@@ -111,15 +110,13 @@ static int nfp_bpf_setup_tc_block_cb(enum tc_setup_type type,
return -EOPNOTSUPP;
}
- skip_sw = !!(cls_bpf->gen_flags & TCA_CLS_FLAGS_SKIP_SW);
-
switch (cls_bpf->command) {
case TC_CLSBPF_REPLACE:
- return nfp_net_bpf_offload(nn, cls_bpf->prog, true, !skip_sw);
+ return nfp_net_bpf_offload(nn, cls_bpf->prog, true);
case TC_CLSBPF_ADD:
- return nfp_net_bpf_offload(nn, cls_bpf->prog, false, !skip_sw);
+ return nfp_net_bpf_offload(nn, cls_bpf->prog, false);
case TC_CLSBPF_DESTROY:
- return nfp_net_bpf_offload(nn, NULL, true, !skip_sw);
+ return nfp_net_bpf_offload(nn, NULL, true);
default:
return -EOPNOTSUPP;
}
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.h b/drivers/net/ethernet/netronome/nfp/bpf/main.h
index 6dddab95d57a..df56f40fea7c 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.h
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.h
@@ -183,6 +183,6 @@ int nfp_prog_verify(struct nfp_prog *nfp_prog, struct bpf_prog *prog);
struct nfp_net;
int nfp_net_bpf_offload(struct nfp_net *nn, struct bpf_prog *prog,
- bool old_prog, bool sw_fallback);
+ bool old_prog);
#endif
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
index c09efa1a9649..f4b9a46c844d 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
@@ -94,14 +94,11 @@ nfp_net_bpf_offload_prepare(struct nfp_net *nn, struct bpf_prog *prog,
}
static void
-nfp_net_bpf_load_and_start(struct nfp_net *nn, bool sw_fallback,
- void *code, dma_addr_t dma_addr,
- unsigned int code_sz, unsigned int n_instr)
+nfp_net_bpf_load(struct nfp_net *nn, void *code, dma_addr_t dma_addr,
+ unsigned int code_sz, unsigned int n_instr)
{
int err;
- nn->dp.bpf_offload_skip_sw = !sw_fallback;
-
nn_writew(nn, NFP_NET_CFG_BPF_SIZE, n_instr);
nn_writeq(nn, NFP_NET_CFG_BPF_ADDR, dma_addr);
@@ -110,14 +107,19 @@ nfp_net_bpf_load_and_start(struct nfp_net *nn, bool sw_fallback,
if (err)
nn_err(nn, "FW command error while loading BPF: %d\n", err);
+ dma_free_coherent(nn->dp.dev, code_sz, code, dma_addr);
+}
+
+static void nfp_net_bpf_start(struct nfp_net *nn)
+{
+ int err;
+
/* Enable passing packets through BPF function */
nn->dp.ctrl |= NFP_NET_CFG_CTRL_BPF;
nn_writel(nn, NFP_NET_CFG_CTRL, nn->dp.ctrl);
err = nfp_net_reconfig(nn, NFP_NET_CFG_UPDATE_GEN);
if (err)
nn_err(nn, "FW command error while enabling BPF: %d\n", err);
-
- dma_free_coherent(nn->dp.dev, code_sz, code, dma_addr);
}
static int nfp_net_bpf_stop(struct nfp_net *nn)
@@ -127,13 +129,12 @@ static int nfp_net_bpf_stop(struct nfp_net *nn)
nn->dp.ctrl &= ~NFP_NET_CFG_CTRL_BPF;
nn_writel(nn, NFP_NET_CFG_CTRL, nn->dp.ctrl);
- nn->dp.bpf_offload_skip_sw = 0;
return nfp_net_reconfig(nn, NFP_NET_CFG_UPDATE_GEN);
}
int nfp_net_bpf_offload(struct nfp_net *nn, struct bpf_prog *prog,
- bool old_prog, bool sw_fallback)
+ bool old_prog)
{
struct nfp_bpf_result res;
dma_addr_t dma_addr;
@@ -141,37 +142,34 @@ int nfp_net_bpf_offload(struct nfp_net *nn, struct bpf_prog *prog,
void *code;
int err;
- /* There is nothing stopping us from implementing seamless
- * replace but the simple method of loading I adopted in
- * the firmware does not handle atomic replace (i.e. we have to
- * stop the BPF offload and re-enable it). Leaking-in a few
- * frames which didn't have BPF applied in the hardware should
- * be fine if software fallback is available, though.
- */
- if (prog && old_prog && nn->dp.bpf_offload_skip_sw)
- return -EBUSY;
+ if (prog && old_prog) {
+ u8 cap;
+
+ cap = nn_readb(nn, NFP_NET_CFG_BPF_CAP);
+ if (!(cap & NFP_NET_BPF_CAP_RELO)) {
+ nn_err(nn, "FW does not support live reload\n");
+ return -EBUSY;
+ }
+ }
/* Something else is loaded, different program type? */
if (!old_prog && nn->dp.ctrl & NFP_NET_CFG_CTRL_BPF)
return -EBUSY;
- max_instr = nn_readw(nn, NFP_NET_CFG_BPF_MAX_LEN);
- code = NULL;
+ if (old_prog && !prog)
+ return nfp_net_bpf_stop(nn);
- if (prog) {
- err = nfp_net_bpf_offload_prepare(nn, prog, &res, &code,
- &dma_addr, max_instr);
- if (err)
- return err;
- }
+ max_instr = nn_readw(nn, NFP_NET_CFG_BPF_MAX_LEN);
- if (old_prog)
- nfp_net_bpf_stop(nn);
+ err = nfp_net_bpf_offload_prepare(nn, prog, &res, &code, &dma_addr,
+ max_instr);
+ if (err)
+ return err;
- if (prog)
- nfp_net_bpf_load_and_start(nn, sw_fallback, code,
- dma_addr, max_instr * sizeof(u64),
- res.n_instr);
+ nfp_net_bpf_load(nn, code, dma_addr, max_instr * sizeof(u64),
+ res.n_instr);
+ if (!old_prog)
+ nfp_net_bpf_start(nn);
return 0;
}
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net.h b/drivers/net/ethernet/netronome/nfp/nfp_net.h
index 3d411f0d15b6..7f9857c276b1 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net.h
@@ -476,7 +476,6 @@ struct nfp_stat_pair {
* @dev: Backpointer to struct device
* @netdev: Backpointer to net_device structure
* @is_vf: Is the driver attached to a VF?
- * @bpf_offload_skip_sw: Offloaded BPF program will not be rerun by cls_bpf
* @bpf_offload_xdp: Offloaded BPF program is XDP
* @chained_metadata_format: Firemware will use new metadata format
* @rx_dma_dir: Mapping direction for RX buffers
@@ -502,7 +501,6 @@ struct nfp_net_dp {
struct net_device *netdev;
u8 is_vf:1;
- u8 bpf_offload_skip_sw:1;
u8 bpf_offload_xdp:1;
u8 chained_metadata_format:1;
--
2.14.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH net-next v2 12/15] nfp: bpf: move program prepare and free into offload.c
2017-11-03 20:56 [PATCH net-next v2 00/15] bpf: add offload as a first class citizen Jakub Kicinski
` (10 preceding siblings ...)
2017-11-03 20:56 ` [PATCH net-next v2 11/15] nfp: bpf: require seamless reload for program replace Jakub Kicinski
@ 2017-11-03 20:56 ` Jakub Kicinski
2017-11-03 20:56 ` [PATCH net-next v2 13/15] nfp: bpf: move translation prepare to offload.c Jakub Kicinski
` (3 subsequent siblings)
15 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2017-11-03 20:56 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, Jakub Kicinski
Most of offload/translation prepare logic will be moved to
offload.c. To help git generate more reasonable diffs
move nfp_prog_prepare() and nfp_prog_free() functions
there as a first step.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
drivers/net/ethernet/netronome/nfp/bpf/jit.c | 33 ------------------------
drivers/net/ethernet/netronome/nfp/bpf/main.h | 5 ++++
drivers/net/ethernet/netronome/nfp/bpf/offload.c | 33 ++++++++++++++++++++++++
3 files changed, 38 insertions(+), 33 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/jit.c b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
index ff150c27f411..2eddbb45fd60 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/jit.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
@@ -77,17 +77,6 @@ nfp_meta_has_prev(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
return meta->l.prev != &nfp_prog->insns;
}
-static void nfp_prog_free(struct nfp_prog *nfp_prog)
-{
- struct nfp_insn_meta *meta, *tmp;
-
- list_for_each_entry_safe(meta, tmp, &nfp_prog->insns, l) {
- list_del(&meta->l);
- kfree(meta);
- }
- kfree(nfp_prog);
-}
-
static void nfp_prog_push(struct nfp_prog *nfp_prog, u64 insn)
{
if (nfp_prog->__prog_alloc_len == nfp_prog->prog_len) {
@@ -2127,28 +2116,6 @@ static int nfp_translate(struct nfp_prog *nfp_prog)
return nfp_fixup_branches(nfp_prog);
}
-static int
-nfp_prog_prepare(struct nfp_prog *nfp_prog, const struct bpf_insn *prog,
- unsigned int cnt)
-{
- unsigned int i;
-
- for (i = 0; i < cnt; i++) {
- struct nfp_insn_meta *meta;
-
- meta = kzalloc(sizeof(*meta), GFP_KERNEL);
- if (!meta)
- return -ENOMEM;
-
- meta->insn = prog[i];
- meta->n = i;
-
- list_add_tail(&meta->l, &nfp_prog->insns);
- }
-
- return 0;
-}
-
/* --- Optimizations --- */
static void nfp_bpf_opt_reg_init(struct nfp_prog *nfp_prog)
{
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.h b/drivers/net/ethernet/netronome/nfp/bpf/main.h
index df56f40fea7c..b77231a134b9 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.h
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.h
@@ -173,6 +173,11 @@ struct nfp_bpf_result {
unsigned int n_instr;
};
+int
+nfp_prog_prepare(struct nfp_prog *nfp_prog, const struct bpf_insn *prog,
+ unsigned int cnt);
+void nfp_prog_free(struct nfp_prog *nfp_prog);
+
int
nfp_bpf_jit(struct bpf_prog *filter, void *prog,
unsigned int prog_start, unsigned int prog_done,
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
index f4b9a46c844d..3eeee200051e 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
@@ -51,6 +51,39 @@
#include "../nfp_net_ctrl.h"
#include "../nfp_net.h"
+int
+nfp_prog_prepare(struct nfp_prog *nfp_prog, const struct bpf_insn *prog,
+ unsigned int cnt)
+{
+ unsigned int i;
+
+ for (i = 0; i < cnt; i++) {
+ struct nfp_insn_meta *meta;
+
+ meta = kzalloc(sizeof(*meta), GFP_KERNEL);
+ if (!meta)
+ return -ENOMEM;
+
+ meta->insn = prog[i];
+ meta->n = i;
+
+ list_add_tail(&meta->l, &nfp_prog->insns);
+ }
+
+ return 0;
+}
+
+void nfp_prog_free(struct nfp_prog *nfp_prog)
+{
+ struct nfp_insn_meta *meta, *tmp;
+
+ list_for_each_entry_safe(meta, tmp, &nfp_prog->insns, l) {
+ list_del(&meta->l);
+ kfree(meta);
+ }
+ kfree(nfp_prog);
+}
+
static int
nfp_net_bpf_offload_prepare(struct nfp_net *nn, struct bpf_prog *prog,
struct nfp_bpf_result *res,
--
2.14.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH net-next v2 13/15] nfp: bpf: move translation prepare to offload.c
2017-11-03 20:56 [PATCH net-next v2 00/15] bpf: add offload as a first class citizen Jakub Kicinski
` (11 preceding siblings ...)
2017-11-03 20:56 ` [PATCH net-next v2 12/15] nfp: bpf: move program prepare and free into offload.c Jakub Kicinski
@ 2017-11-03 20:56 ` Jakub Kicinski
2017-11-03 20:56 ` [PATCH net-next v2 14/15] nfp: bpf: move to new BPF program offload infrastructure Jakub Kicinski
` (2 subsequent siblings)
15 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2017-11-03 20:56 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, Jakub Kicinski
struct nfp_prog is currently only used internally by the translator.
This means there is a lot of parameter passing going on, between
the translator and different stages of offload. Simplify things
by allocating nfp_prog in offload.c already.
We will now use kmalloc() to allocate the program area and only
DMA map it for the time of loading (instead of allocating DMA
coherent memory upfront).
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
drivers/net/ethernet/netronome/nfp/bpf/jit.c | 43 ++------
drivers/net/ethernet/netronome/nfp/bpf/main.h | 14 +--
drivers/net/ethernet/netronome/nfp/bpf/offload.c | 128 +++++++++++++++--------
3 files changed, 94 insertions(+), 91 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/jit.c b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
index 2eddbb45fd60..eae7a137a7a8 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/jit.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
@@ -2245,58 +2245,27 @@ static int nfp_bpf_ustore_calc(struct nfp_prog *nfp_prog, __le64 *ustore)
/**
* nfp_bpf_jit() - translate BPF code into NFP assembly
+ * @nfp_prog: nfp_prog prepared based on @filter
* @filter: kernel BPF filter struct
- * @prog_mem: memory to store assembler instructions
- * @prog_start: offset of the first instruction when loaded
- * @prog_done: where to jump on exit
- * @prog_sz: size of @prog_mem in instructions
- * @res: achieved parameters of translation results
*/
-int
-nfp_bpf_jit(struct bpf_prog *filter, void *prog_mem,
- unsigned int prog_start, unsigned int prog_done,
- unsigned int prog_sz, struct nfp_bpf_result *res)
+int nfp_bpf_jit(struct nfp_prog *nfp_prog, struct bpf_prog *filter)
{
- struct nfp_prog *nfp_prog;
int ret;
- nfp_prog = kzalloc(sizeof(*nfp_prog), GFP_KERNEL);
- if (!nfp_prog)
- return -ENOMEM;
-
- INIT_LIST_HEAD(&nfp_prog->insns);
- nfp_prog->type = filter->type;
- nfp_prog->start_off = prog_start;
- nfp_prog->tgt_done = prog_done;
-
- ret = nfp_prog_prepare(nfp_prog, filter->insnsi, filter->len);
- if (ret)
- goto out;
-
ret = nfp_prog_verify(nfp_prog, filter);
if (ret)
- goto out;
+ return ret;
ret = nfp_bpf_optimize(nfp_prog);
if (ret)
- goto out;
-
- nfp_prog->prog = prog_mem;
- nfp_prog->__prog_alloc_len = prog_sz;
+ return ret;
ret = nfp_translate(nfp_prog);
if (ret) {
pr_err("Translation failed with error %d (translated: %u)\n",
ret, nfp_prog->n_translated);
- ret = -EINVAL;
- goto out;
+ return -EINVAL;
}
- ret = nfp_bpf_ustore_calc(nfp_prog, (__force __le64 *)prog_mem);
-
- res->n_instr = nfp_prog->prog_len;
-out:
- nfp_prog_free(nfp_prog);
-
- return ret;
+ return nfp_bpf_ustore_calc(nfp_prog, (__force __le64 *)nfp_prog->prog);
}
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.h b/drivers/net/ethernet/netronome/nfp/bpf/main.h
index b77231a134b9..36b4eda2d3f8 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.h
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.h
@@ -169,19 +169,7 @@ struct nfp_prog {
struct list_head insns;
};
-struct nfp_bpf_result {
- unsigned int n_instr;
-};
-
-int
-nfp_prog_prepare(struct nfp_prog *nfp_prog, const struct bpf_insn *prog,
- unsigned int cnt);
-void nfp_prog_free(struct nfp_prog *nfp_prog);
-
-int
-nfp_bpf_jit(struct bpf_prog *filter, void *prog,
- unsigned int prog_start, unsigned int prog_done,
- unsigned int prog_sz, struct nfp_bpf_result *res);
+int nfp_bpf_jit(struct nfp_prog *nfp_prog, struct bpf_prog *filter);
int nfp_prog_verify(struct nfp_prog *nfp_prog, struct bpf_prog *prog);
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
index 3eeee200051e..c5546c0e87d8 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
@@ -51,7 +51,7 @@
#include "../nfp_net_ctrl.h"
#include "../nfp_net.h"
-int
+static int
nfp_prog_prepare(struct nfp_prog *nfp_prog, const struct bpf_insn *prog,
unsigned int cnt)
{
@@ -73,7 +73,7 @@ nfp_prog_prepare(struct nfp_prog *nfp_prog, const struct bpf_insn *prog,
return 0;
}
-void nfp_prog_free(struct nfp_prog *nfp_prog)
+static void nfp_prog_free(struct nfp_prog *nfp_prog)
{
struct nfp_insn_meta *meta, *tmp;
@@ -84,25 +84,36 @@ void nfp_prog_free(struct nfp_prog *nfp_prog)
kfree(nfp_prog);
}
-static int
-nfp_net_bpf_offload_prepare(struct nfp_net *nn, struct bpf_prog *prog,
- struct nfp_bpf_result *res,
- void **code, dma_addr_t *dma_addr, u16 max_instr)
+static struct nfp_prog *nfp_bpf_verifier_prep(struct bpf_prog *prog)
{
- unsigned int code_sz = max_instr * sizeof(u64);
- unsigned int stack_size;
- u16 start_off, done_off;
- unsigned int max_mtu;
+ struct nfp_prog *nfp_prog;
int ret;
- max_mtu = nn_readb(nn, NFP_NET_CFG_BPF_INL_MTU) * 64 - 32;
- if (max_mtu < nn->dp.netdev->mtu) {
- nn_info(nn, "BPF offload not supported with MTU larger than HW packet split boundary\n");
- return -EOPNOTSUPP;
- }
+ nfp_prog = kzalloc(sizeof(*nfp_prog), GFP_KERNEL);
+ if (!nfp_prog)
+ return NULL;
+
+ INIT_LIST_HEAD(&nfp_prog->insns);
+ nfp_prog->type = prog->type;
- start_off = nn_readw(nn, NFP_NET_CFG_BPF_START);
- done_off = nn_readw(nn, NFP_NET_CFG_BPF_DONE);
+ ret = nfp_prog_prepare(nfp_prog, prog->insnsi, prog->len);
+ if (ret)
+ goto err_free;
+
+ return nfp_prog;
+
+err_free:
+ nfp_prog_free(nfp_prog);
+
+ return NULL;
+}
+
+static int
+nfp_bpf_translate(struct nfp_net *nn, struct nfp_prog *nfp_prog,
+ struct bpf_prog *prog)
+{
+ unsigned int stack_size;
+ unsigned int max_instr;
stack_size = nn_readb(nn, NFP_NET_CFG_BPF_STACK_SZ) * 64;
if (prog->aux->stack_depth > stack_size) {
@@ -111,28 +122,68 @@ nfp_net_bpf_offload_prepare(struct nfp_net *nn, struct bpf_prog *prog,
return -EOPNOTSUPP;
}
- *code = dma_zalloc_coherent(nn->dp.dev, code_sz, dma_addr, GFP_KERNEL);
- if (!*code)
+ nfp_prog->stack_depth = prog->aux->stack_depth;
+ nfp_prog->start_off = nn_readw(nn, NFP_NET_CFG_BPF_START);
+ nfp_prog->tgt_done = nn_readw(nn, NFP_NET_CFG_BPF_DONE);
+
+ max_instr = nn_readw(nn, NFP_NET_CFG_BPF_MAX_LEN);
+ nfp_prog->__prog_alloc_len = max_instr * sizeof(u64);
+
+ nfp_prog->prog = kmalloc(nfp_prog->__prog_alloc_len, GFP_KERNEL);
+ if (!nfp_prog->prog)
return -ENOMEM;
- ret = nfp_bpf_jit(prog, *code, start_off, done_off, max_instr, res);
- if (ret)
- goto out;
+ return nfp_bpf_jit(nfp_prog, prog);
+}
+
+static void nfp_bpf_destroy(struct nfp_prog *nfp_prog)
+{
+ kfree(nfp_prog->prog);
+ nfp_prog_free(nfp_prog);
+}
+
+static struct nfp_prog *
+nfp_net_bpf_offload_prepare(struct nfp_net *nn, struct bpf_prog *prog,
+ dma_addr_t *dma_addr)
+{
+ struct nfp_prog *nfp_prog;
+ unsigned int max_mtu;
+ int err;
+
+ max_mtu = nn_readb(nn, NFP_NET_CFG_BPF_INL_MTU) * 64 - 32;
+ if (max_mtu < nn->dp.netdev->mtu) {
+ nn_info(nn, "BPF offload not supported with MTU larger than HW packet split boundary\n");
+ return NULL;
+ }
+
+ nfp_prog = nfp_bpf_verifier_prep(prog);
+ if (!nfp_prog)
+ return NULL;
+
+ err = nfp_bpf_translate(nn, nfp_prog, prog);
+ if (err)
+ goto err_destroy_prog;
+
+ *dma_addr = dma_map_single(nn->dp.dev, nfp_prog->prog,
+ nfp_prog->prog_len * sizeof(u64),
+ DMA_TO_DEVICE);
+ if (dma_mapping_error(nn->dp.dev, *dma_addr))
+ goto err_destroy_prog;
return 0;
-out:
- dma_free_coherent(nn->dp.dev, code_sz, *code, *dma_addr);
- return ret;
+err_destroy_prog:
+ nfp_bpf_destroy(nfp_prog);
+ return NULL;
}
static void
-nfp_net_bpf_load(struct nfp_net *nn, void *code, dma_addr_t dma_addr,
- unsigned int code_sz, unsigned int n_instr)
+nfp_net_bpf_load(struct nfp_net *nn, struct nfp_prog *nfp_prog,
+ dma_addr_t dma_addr)
{
int err;
- nn_writew(nn, NFP_NET_CFG_BPF_SIZE, n_instr);
+ nn_writew(nn, NFP_NET_CFG_BPF_SIZE, nfp_prog->prog_len);
nn_writeq(nn, NFP_NET_CFG_BPF_ADDR, dma_addr);
/* Load up the JITed code */
@@ -140,7 +191,9 @@ nfp_net_bpf_load(struct nfp_net *nn, void *code, dma_addr_t dma_addr,
if (err)
nn_err(nn, "FW command error while loading BPF: %d\n", err);
- dma_free_coherent(nn->dp.dev, code_sz, code, dma_addr);
+ dma_unmap_single(nn->dp.dev, dma_addr, nfp_prog->prog_len * sizeof(u64),
+ DMA_TO_DEVICE);
+ nfp_bpf_destroy(nfp_prog);
}
static void nfp_net_bpf_start(struct nfp_net *nn)
@@ -169,11 +222,8 @@ static int nfp_net_bpf_stop(struct nfp_net *nn)
int nfp_net_bpf_offload(struct nfp_net *nn, struct bpf_prog *prog,
bool old_prog)
{
- struct nfp_bpf_result res;
+ struct nfp_prog *nfp_prog;
dma_addr_t dma_addr;
- u16 max_instr;
- void *code;
- int err;
if (prog && old_prog) {
u8 cap;
@@ -192,15 +242,11 @@ int nfp_net_bpf_offload(struct nfp_net *nn, struct bpf_prog *prog,
if (old_prog && !prog)
return nfp_net_bpf_stop(nn);
- max_instr = nn_readw(nn, NFP_NET_CFG_BPF_MAX_LEN);
-
- err = nfp_net_bpf_offload_prepare(nn, prog, &res, &code, &dma_addr,
- max_instr);
- if (err)
- return err;
+ nfp_prog = nfp_net_bpf_offload_prepare(nn, prog, &dma_addr);
+ if (!nfp_prog)
+ return -EINVAL;
- nfp_net_bpf_load(nn, code, dma_addr, max_instr * sizeof(u64),
- res.n_instr);
+ nfp_net_bpf_load(nn, nfp_prog, dma_addr);
if (!old_prog)
nfp_net_bpf_start(nn);
--
2.14.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH net-next v2 14/15] nfp: bpf: move to new BPF program offload infrastructure
2017-11-03 20:56 [PATCH net-next v2 00/15] bpf: add offload as a first class citizen Jakub Kicinski
` (12 preceding siblings ...)
2017-11-03 20:56 ` [PATCH net-next v2 13/15] nfp: bpf: move translation prepare to offload.c Jakub Kicinski
@ 2017-11-03 20:56 ` Jakub Kicinski
2017-11-03 20:56 ` [PATCH net-next v2 15/15] bpf: remove old offload/analyzer Jakub Kicinski
2017-11-05 13:28 ` [PATCH net-next v2 00/15] bpf: add offload as a first class citizen David Miller
15 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2017-11-03 20:56 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, Jakub Kicinski
Following steps are taken in the driver to offload an XDP program:
XDP_SETUP_PROG:
* prepare:
- allocate program state;
- run verifier (bpf_analyzer());
- run translation;
* load:
- stop old program if needed;
- load program;
- enable BPF if not enabled;
* clean up:
- free program image.
With new infrastructure the flow will look like this:
BPF_OFFLOAD_VERIFIER_PREP:
- allocate program state;
BPF_OFFLOAD_TRANSLATE:
- run translation;
XDP_SETUP_PROG:
- stop old program if needed;
- load program;
- enable BPF if not enabled;
BPF_OFFLOAD_DESTROY:
- free program image.
Take advantage of the new infrastructure. Allocation of driver
metadata has to be moved from jit.c to offload.c since it's now
done at a different stage. Since there is no separate driver
private data for verification step, move temporary nfp_meta
pointer into nfp_prog. We will now use user space context
offsets.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
drivers/net/ethernet/netronome/nfp/bpf/jit.c | 35 ++++-----
drivers/net/ethernet/netronome/nfp/bpf/main.c | 4 +
drivers/net/ethernet/netronome/nfp/bpf/main.h | 15 +++-
drivers/net/ethernet/netronome/nfp/bpf/offload.c | 85 ++++++++++------------
drivers/net/ethernet/netronome/nfp/bpf/verifier.c | 43 ++---------
drivers/net/ethernet/netronome/nfp/nfp_app.h | 37 ++++++++++
.../net/ethernet/netronome/nfp/nfp_net_common.c | 8 ++
7 files changed, 121 insertions(+), 106 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/jit.c b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
index eae7a137a7a8..995e95410b11 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/jit.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
@@ -1427,19 +1427,18 @@ static int mem_ldx_skb(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta,
swreg dst = reg_both(meta->insn.dst_reg * 2);
switch (meta->insn.off) {
- case offsetof(struct sk_buff, len):
- if (size != FIELD_SIZEOF(struct sk_buff, len))
+ case offsetof(struct __sk_buff, len):
+ if (size != FIELD_SIZEOF(struct __sk_buff, len))
return -EOPNOTSUPP;
wrp_mov(nfp_prog, dst, plen_reg(nfp_prog));
break;
- case offsetof(struct sk_buff, data):
- if (size != sizeof(void *))
+ case offsetof(struct __sk_buff, data):
+ if (size != FIELD_SIZEOF(struct __sk_buff, data))
return -EOPNOTSUPP;
wrp_mov(nfp_prog, dst, pptr_reg(nfp_prog));
break;
- case offsetof(struct sk_buff, cb) +
- offsetof(struct bpf_skb_data_end, data_end):
- if (size != sizeof(void *))
+ case offsetof(struct __sk_buff, data_end):
+ if (size != FIELD_SIZEOF(struct __sk_buff, data_end))
return -EOPNOTSUPP;
emit_alu(nfp_prog, dst,
plen_reg(nfp_prog), ALU_OP_ADD, pptr_reg(nfp_prog));
@@ -1458,14 +1457,15 @@ static int mem_ldx_xdp(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta,
{
swreg dst = reg_both(meta->insn.dst_reg * 2);
- if (size != sizeof(void *))
- return -EINVAL;
-
switch (meta->insn.off) {
- case offsetof(struct xdp_buff, data):
+ case offsetof(struct xdp_md, data):
+ if (size != FIELD_SIZEOF(struct xdp_md, data))
+ return -EOPNOTSUPP;
wrp_mov(nfp_prog, dst, pptr_reg(nfp_prog));
break;
- case offsetof(struct xdp_buff, data_end):
+ case offsetof(struct xdp_md, data_end):
+ if (size != FIELD_SIZEOF(struct xdp_md, data_end))
+ return -EOPNOTSUPP;
emit_alu(nfp_prog, dst,
plen_reg(nfp_prog), ALU_OP_ADD, pptr_reg(nfp_prog));
break;
@@ -2243,19 +2243,10 @@ static int nfp_bpf_ustore_calc(struct nfp_prog *nfp_prog, __le64 *ustore)
return 0;
}
-/**
- * nfp_bpf_jit() - translate BPF code into NFP assembly
- * @nfp_prog: nfp_prog prepared based on @filter
- * @filter: kernel BPF filter struct
- */
-int nfp_bpf_jit(struct nfp_prog *nfp_prog, struct bpf_prog *filter)
+int nfp_bpf_jit(struct nfp_prog *nfp_prog)
{
int ret;
- ret = nfp_prog_verify(nfp_prog, filter);
- if (ret)
- return ret;
-
ret = nfp_bpf_optimize(nfp_prog);
if (ret)
return ret;
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.c b/drivers/net/ethernet/netronome/nfp/bpf/main.c
index 7ae7528cd96b..e379b78e86ef 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.c
@@ -173,4 +173,8 @@ const struct nfp_app_type app_bpf = {
.setup_tc = nfp_bpf_setup_tc,
.tc_busy = nfp_bpf_tc_busy,
.xdp_offload = nfp_bpf_xdp_offload,
+
+ .bpf_verifier_prep = nfp_bpf_verifier_prep,
+ .bpf_translate = nfp_bpf_translate,
+ .bpf_destroy = nfp_bpf_destroy,
};
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.h b/drivers/net/ethernet/netronome/nfp/bpf/main.h
index 36b4eda2d3f8..082a15f6dfb5 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.h
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.h
@@ -139,6 +139,7 @@ static inline u8 mbpf_mode(const struct nfp_insn_meta *meta)
* @prog: machine code
* @prog_len: number of valid instructions in @prog array
* @__prog_alloc_len: alloc size of @prog array
+ * @verifier_meta: temporary storage for verifier's insn meta
* @type: BPF program type
* @start_off: address of the first instruction in the memory
* @tgt_out: jump target for normal exit
@@ -154,6 +155,8 @@ struct nfp_prog {
unsigned int prog_len;
unsigned int __prog_alloc_len;
+ struct nfp_insn_meta *verifier_meta;
+
enum bpf_prog_type type;
unsigned int start_off;
@@ -169,13 +172,21 @@ struct nfp_prog {
struct list_head insns;
};
-int nfp_bpf_jit(struct nfp_prog *nfp_prog, struct bpf_prog *filter);
+int nfp_bpf_jit(struct nfp_prog *prog);
-int nfp_prog_verify(struct nfp_prog *nfp_prog, struct bpf_prog *prog);
+extern const struct bpf_ext_analyzer_ops nfp_bpf_analyzer_ops;
+struct netdev_bpf;
+struct nfp_app;
struct nfp_net;
int nfp_net_bpf_offload(struct nfp_net *nn, struct bpf_prog *prog,
bool old_prog);
+int nfp_bpf_verifier_prep(struct nfp_app *app, struct nfp_net *nn,
+ struct netdev_bpf *bpf);
+int nfp_bpf_translate(struct nfp_app *app, struct nfp_net *nn,
+ struct bpf_prog *prog);
+int nfp_bpf_destroy(struct nfp_app *app, struct nfp_net *nn,
+ struct bpf_prog *prog);
#endif
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
index c5546c0e87d8..b6cee71f49d3 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
@@ -84,14 +84,17 @@ static void nfp_prog_free(struct nfp_prog *nfp_prog)
kfree(nfp_prog);
}
-static struct nfp_prog *nfp_bpf_verifier_prep(struct bpf_prog *prog)
+int nfp_bpf_verifier_prep(struct nfp_app *app, struct nfp_net *nn,
+ struct netdev_bpf *bpf)
{
+ struct bpf_prog *prog = bpf->verifier.prog;
struct nfp_prog *nfp_prog;
int ret;
nfp_prog = kzalloc(sizeof(*nfp_prog), GFP_KERNEL);
if (!nfp_prog)
- return NULL;
+ return -ENOMEM;
+ prog->aux->offload->dev_priv = nfp_prog;
INIT_LIST_HEAD(&nfp_prog->insns);
nfp_prog->type = prog->type;
@@ -100,18 +103,21 @@ static struct nfp_prog *nfp_bpf_verifier_prep(struct bpf_prog *prog)
if (ret)
goto err_free;
- return nfp_prog;
+ nfp_prog->verifier_meta = nfp_prog_first_meta(nfp_prog);
+ bpf->verifier.ops = &nfp_bpf_analyzer_ops;
+
+ return 0;
err_free:
nfp_prog_free(nfp_prog);
- return NULL;
+ return ret;
}
-static int
-nfp_bpf_translate(struct nfp_net *nn, struct nfp_prog *nfp_prog,
- struct bpf_prog *prog)
+int nfp_bpf_translate(struct nfp_app *app, struct nfp_net *nn,
+ struct bpf_prog *prog)
{
+ struct nfp_prog *nfp_prog = prog->aux->offload->dev_priv;
unsigned int stack_size;
unsigned int max_instr;
@@ -133,55 +139,38 @@ nfp_bpf_translate(struct nfp_net *nn, struct nfp_prog *nfp_prog,
if (!nfp_prog->prog)
return -ENOMEM;
- return nfp_bpf_jit(nfp_prog, prog);
+ return nfp_bpf_jit(nfp_prog);
}
-static void nfp_bpf_destroy(struct nfp_prog *nfp_prog)
+int nfp_bpf_destroy(struct nfp_app *app, struct nfp_net *nn,
+ struct bpf_prog *prog)
{
+ struct nfp_prog *nfp_prog = prog->aux->offload->dev_priv;
+
kfree(nfp_prog->prog);
nfp_prog_free(nfp_prog);
+
+ return 0;
}
-static struct nfp_prog *
-nfp_net_bpf_offload_prepare(struct nfp_net *nn, struct bpf_prog *prog,
- dma_addr_t *dma_addr)
+static int nfp_net_bpf_load(struct nfp_net *nn, struct bpf_prog *prog)
{
- struct nfp_prog *nfp_prog;
+ struct nfp_prog *nfp_prog = prog->aux->offload->dev_priv;
unsigned int max_mtu;
+ dma_addr_t dma_addr;
int err;
max_mtu = nn_readb(nn, NFP_NET_CFG_BPF_INL_MTU) * 64 - 32;
if (max_mtu < nn->dp.netdev->mtu) {
nn_info(nn, "BPF offload not supported with MTU larger than HW packet split boundary\n");
- return NULL;
+ return -EOPNOTSUPP;
}
- nfp_prog = nfp_bpf_verifier_prep(prog);
- if (!nfp_prog)
- return NULL;
-
- err = nfp_bpf_translate(nn, nfp_prog, prog);
- if (err)
- goto err_destroy_prog;
-
- *dma_addr = dma_map_single(nn->dp.dev, nfp_prog->prog,
- nfp_prog->prog_len * sizeof(u64),
- DMA_TO_DEVICE);
- if (dma_mapping_error(nn->dp.dev, *dma_addr))
- goto err_destroy_prog;
-
- return 0;
-
-err_destroy_prog:
- nfp_bpf_destroy(nfp_prog);
- return NULL;
-}
-
-static void
-nfp_net_bpf_load(struct nfp_net *nn, struct nfp_prog *nfp_prog,
- dma_addr_t dma_addr)
-{
- int err;
+ dma_addr = dma_map_single(nn->dp.dev, nfp_prog->prog,
+ nfp_prog->prog_len * sizeof(u64),
+ DMA_TO_DEVICE);
+ if (dma_mapping_error(nn->dp.dev, dma_addr))
+ return -ENOMEM;
nn_writew(nn, NFP_NET_CFG_BPF_SIZE, nfp_prog->prog_len);
nn_writeq(nn, NFP_NET_CFG_BPF_ADDR, dma_addr);
@@ -193,7 +182,8 @@ nfp_net_bpf_load(struct nfp_net *nn, struct nfp_prog *nfp_prog,
dma_unmap_single(nn->dp.dev, dma_addr, nfp_prog->prog_len * sizeof(u64),
DMA_TO_DEVICE);
- nfp_bpf_destroy(nfp_prog);
+
+ return err;
}
static void nfp_net_bpf_start(struct nfp_net *nn)
@@ -222,8 +212,10 @@ static int nfp_net_bpf_stop(struct nfp_net *nn)
int nfp_net_bpf_offload(struct nfp_net *nn, struct bpf_prog *prog,
bool old_prog)
{
- struct nfp_prog *nfp_prog;
- dma_addr_t dma_addr;
+ int err;
+
+ if (prog && !prog->aux->offload)
+ return -EINVAL;
if (prog && old_prog) {
u8 cap;
@@ -242,11 +234,10 @@ int nfp_net_bpf_offload(struct nfp_net *nn, struct bpf_prog *prog,
if (old_prog && !prog)
return nfp_net_bpf_stop(nn);
- nfp_prog = nfp_net_bpf_offload_prepare(nn, prog, &dma_addr);
- if (!nfp_prog)
- return -EINVAL;
+ err = nfp_net_bpf_load(nn, prog);
+ if (err)
+ return err;
- nfp_net_bpf_load(nn, nfp_prog, dma_addr);
if (!old_prog)
nfp_net_bpf_start(nn);
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/verifier.c b/drivers/net/ethernet/netronome/nfp/bpf/verifier.c
index 4f31bdefd331..8d43491ddd6b 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/verifier.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/verifier.c
@@ -40,12 +40,6 @@
#include "main.h"
-/* Analyzer/verifier definitions */
-struct nfp_bpf_analyzer_priv {
- struct nfp_prog *prog;
- struct nfp_insn_meta *meta;
-};
-
static struct nfp_insn_meta *
nfp_bpf_goto_meta(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta,
unsigned int insn_idx, unsigned int n_insns)
@@ -171,11 +165,11 @@ nfp_bpf_check_ptr(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta,
static int
nfp_verify_insn(struct bpf_verifier_env *env, int insn_idx, int prev_insn_idx)
{
- struct nfp_bpf_analyzer_priv *priv = env->analyzer_priv;
- struct nfp_insn_meta *meta = priv->meta;
+ struct nfp_prog *nfp_prog = env->prog->aux->offload->dev_priv;
+ struct nfp_insn_meta *meta = nfp_prog->verifier_meta;
- meta = nfp_bpf_goto_meta(priv->prog, meta, insn_idx, env->prog->len);
- priv->meta = meta;
+ meta = nfp_bpf_goto_meta(nfp_prog, meta, insn_idx, env->prog->len);
+ nfp_prog->verifier_meta = meta;
if (meta->insn.src_reg >= MAX_BPF_REG ||
meta->insn.dst_reg >= MAX_BPF_REG) {
@@ -184,39 +178,18 @@ nfp_verify_insn(struct bpf_verifier_env *env, int insn_idx, int prev_insn_idx)
}
if (meta->insn.code == (BPF_JMP | BPF_EXIT))
- return nfp_bpf_check_exit(priv->prog, env);
+ return nfp_bpf_check_exit(nfp_prog, env);
if ((meta->insn.code & ~BPF_SIZE_MASK) == (BPF_LDX | BPF_MEM))
- return nfp_bpf_check_ptr(priv->prog, meta, env,
+ return nfp_bpf_check_ptr(nfp_prog, meta, env,
meta->insn.src_reg);
if ((meta->insn.code & ~BPF_SIZE_MASK) == (BPF_STX | BPF_MEM))
- return nfp_bpf_check_ptr(priv->prog, meta, env,
+ return nfp_bpf_check_ptr(nfp_prog, meta, env,
meta->insn.dst_reg);
return 0;
}
-static const struct bpf_ext_analyzer_ops nfp_bpf_analyzer_ops = {
+const struct bpf_ext_analyzer_ops nfp_bpf_analyzer_ops = {
.insn_hook = nfp_verify_insn,
};
-
-int nfp_prog_verify(struct nfp_prog *nfp_prog, struct bpf_prog *prog)
-{
- struct nfp_bpf_analyzer_priv *priv;
- int ret;
-
- nfp_prog->stack_depth = prog->aux->stack_depth;
-
- priv = kzalloc(sizeof(*priv), GFP_KERNEL);
- if (!priv)
- return -ENOMEM;
-
- priv->prog = nfp_prog;
- priv->meta = nfp_prog_first_meta(nfp_prog);
-
- ret = bpf_analyzer(prog, &nfp_bpf_analyzer_ops, priv);
-
- kfree(priv);
-
- return ret;
-}
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_app.h b/drivers/net/ethernet/netronome/nfp/nfp_app.h
index 857bb33020ba..54b67c9b8d5b 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_app.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_app.h
@@ -42,6 +42,7 @@
struct bpf_prog;
struct net_device;
+struct netdev_bpf;
struct pci_dev;
struct sk_buff;
struct sk_buff;
@@ -83,6 +84,9 @@ extern const struct nfp_app_type app_flower;
* @setup_tc: setup TC ndo
* @tc_busy: TC HW offload busy (rules loaded)
* @xdp_offload: offload an XDP program
+ * @bpf_verifier_prep: verifier prep for dev-specific BPF programs
+ * @bpf_translate: translate call for dev-specific BPF programs
+ * @bpf_destroy: destroy for dev-specific BPF programs
* @eswitch_mode_get: get SR-IOV eswitch mode
* @sriov_enable: app-specific sriov initialisation
* @sriov_disable: app-specific sriov clean-up
@@ -118,6 +122,12 @@ struct nfp_app_type {
bool (*tc_busy)(struct nfp_app *app, struct nfp_net *nn);
int (*xdp_offload)(struct nfp_app *app, struct nfp_net *nn,
struct bpf_prog *prog);
+ int (*bpf_verifier_prep)(struct nfp_app *app, struct nfp_net *nn,
+ struct netdev_bpf *bpf);
+ int (*bpf_translate)(struct nfp_app *app, struct nfp_net *nn,
+ struct bpf_prog *prog);
+ int (*bpf_destroy)(struct nfp_app *app, struct nfp_net *nn,
+ struct bpf_prog *prog);
int (*sriov_enable)(struct nfp_app *app, int num_vfs);
void (*sriov_disable)(struct nfp_app *app);
@@ -271,6 +281,33 @@ static inline int nfp_app_xdp_offload(struct nfp_app *app, struct nfp_net *nn,
return app->type->xdp_offload(app, nn, prog);
}
+static inline int
+nfp_app_bpf_verifier_prep(struct nfp_app *app, struct nfp_net *nn,
+ struct netdev_bpf *bpf)
+{
+ if (!app || !app->type->bpf_verifier_prep)
+ return -EOPNOTSUPP;
+ return app->type->bpf_verifier_prep(app, nn, bpf);
+}
+
+static inline int
+nfp_app_bpf_translate(struct nfp_app *app, struct nfp_net *nn,
+ struct bpf_prog *prog)
+{
+ if (!app || !app->type->bpf_translate)
+ return -EOPNOTSUPP;
+ return app->type->bpf_translate(app, nn, prog);
+}
+
+static inline int
+nfp_app_bpf_destroy(struct nfp_app *app, struct nfp_net *nn,
+ struct bpf_prog *prog)
+{
+ if (!app || !app->type->bpf_destroy)
+ return -EOPNOTSUPP;
+ return app->type->bpf_destroy(app, nn, prog);
+}
+
static inline bool nfp_app_ctrl_tx(struct nfp_app *app, struct sk_buff *skb)
{
trace_devlink_hwmsg(priv_to_devlink(app->pf), false, 0,
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index f6c6ad4e8a59..232044b1b7aa 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -3393,6 +3393,14 @@ static int nfp_net_xdp(struct net_device *netdev, struct netdev_bpf *xdp)
xdp->prog_attached = XDP_ATTACHED_HW;
xdp->prog_id = nn->xdp_prog ? nn->xdp_prog->aux->id : 0;
return 0;
+ case BPF_OFFLOAD_VERIFIER_PREP:
+ return nfp_app_bpf_verifier_prep(nn->app, nn, xdp);
+ case BPF_OFFLOAD_TRANSLATE:
+ return nfp_app_bpf_translate(nn->app, nn,
+ xdp->offload.prog);
+ case BPF_OFFLOAD_DESTROY:
+ return nfp_app_bpf_destroy(nn->app, nn,
+ xdp->offload.prog);
default:
return -EINVAL;
}
--
2.14.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH net-next v2 15/15] bpf: remove old offload/analyzer
2017-11-03 20:56 [PATCH net-next v2 00/15] bpf: add offload as a first class citizen Jakub Kicinski
` (13 preceding siblings ...)
2017-11-03 20:56 ` [PATCH net-next v2 14/15] nfp: bpf: move to new BPF program offload infrastructure Jakub Kicinski
@ 2017-11-03 20:56 ` Jakub Kicinski
2017-11-05 13:28 ` [PATCH net-next v2 00/15] bpf: add offload as a first class citizen David Miller
15 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2017-11-03 20:56 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, Jakub Kicinski
Thanks to the ability to load a program for a specific device,
running verifier twice is no longer needed.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
include/linux/bpf_verifier.h | 5 ---
kernel/bpf/verifier.c | 75 --------------------------------------------
net/core/filter.c | 42 -------------------------
3 files changed, 122 deletions(-)
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index e45011dbc02d..07b96aaca256 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -152,9 +152,7 @@ struct bpf_verifier_env {
bool strict_alignment; /* perform strict pointer alignment checks */
struct bpf_verifier_state *cur_state; /* current verifier state */
struct bpf_verifier_state_list **explored_states; /* search pruning optimization */
- const struct bpf_ext_analyzer_ops *analyzer_ops; /* external analyzer ops */
const struct bpf_ext_analyzer_ops *dev_ops; /* device analyzer ops */
- void *analyzer_priv; /* pointer to external analyzer's private data */
struct bpf_map *used_maps[MAX_USED_MAPS]; /* array of map's used by eBPF program */
u32 used_map_cnt; /* number of used maps */
u32 id_gen; /* used to generate unique reg IDs */
@@ -179,7 +177,4 @@ int bpf_prog_offload_verifier_prep(struct bpf_verifier_env *env)
}
#endif
-int bpf_analyzer(struct bpf_prog *prog, const struct bpf_ext_analyzer_ops *ops,
- void *priv);
-
#endif /* _LINUX_BPF_VERIFIER_H */
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 51aabb32ad67..add845fe788a 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -949,9 +949,6 @@ static int check_ctx_access(struct bpf_verifier_env *env, int insn_idx, int off,
*/
*reg_type = info.reg_type;
- if (env->analyzer_ops)
- return 0;
-
env->insn_aux_data[insn_idx].ctx_field_size = info.ctx_field_size;
/* remember the offset of last byte accessed in ctx */
if (env->prog->aux->max_ctx_offset < off + size)
@@ -3736,9 +3733,6 @@ static int is_state_visited(struct bpf_verifier_env *env, int insn_idx)
static int ext_analyzer_insn_hook(struct bpf_verifier_env *env,
int insn_idx, int prev_insn_idx)
{
- if (env->analyzer_ops && env->analyzer_ops->insn_hook)
- return env->analyzer_ops->insn_hook(env, insn_idx,
- prev_insn_idx);
if (env->dev_ops && env->dev_ops->insn_hook)
return env->dev_ops->insn_hook(env, insn_idx, prev_insn_idx);
@@ -4601,72 +4595,3 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr)
kfree(env);
return ret;
}
-
-static const struct bpf_verifier_ops * const bpf_analyzer_ops[] = {
-#ifdef CONFIG_NET
- [BPF_PROG_TYPE_XDP] = &xdp_analyzer_ops,
- [BPF_PROG_TYPE_SCHED_CLS] = &tc_cls_act_analyzer_ops,
-#endif
-};
-
-int bpf_analyzer(struct bpf_prog *prog, const struct bpf_ext_analyzer_ops *ops,
- void *priv)
-{
- struct bpf_verifier_env *env;
- int ret;
-
- if (prog->type >= ARRAY_SIZE(bpf_analyzer_ops) ||
- !bpf_analyzer_ops[prog->type])
- return -EOPNOTSUPP;
-
- env = kzalloc(sizeof(struct bpf_verifier_env), GFP_KERNEL);
- if (!env)
- return -ENOMEM;
-
- env->insn_aux_data = vzalloc(sizeof(struct bpf_insn_aux_data) *
- prog->len);
- ret = -ENOMEM;
- if (!env->insn_aux_data)
- goto err_free_env;
- env->prog = prog;
- env->ops = bpf_analyzer_ops[env->prog->type];
- env->analyzer_ops = ops;
- env->analyzer_priv = priv;
-
- /* grab the mutex to protect few globals used by verifier */
- mutex_lock(&bpf_verifier_lock);
-
- env->strict_alignment = false;
- if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS))
- env->strict_alignment = true;
-
- env->explored_states = kcalloc(env->prog->len,
- sizeof(struct bpf_verifier_state_list *),
- GFP_KERNEL);
- ret = -ENOMEM;
- if (!env->explored_states)
- goto skip_full_check;
-
- ret = check_cfg(env);
- if (ret < 0)
- goto skip_full_check;
-
- env->allow_ptr_leaks = capable(CAP_SYS_ADMIN);
-
- ret = do_check(env);
- if (env->cur_state) {
- free_verifier_state(env->cur_state, true);
- env->cur_state = NULL;
- }
-
-skip_full_check:
- while (!pop_stack(env, NULL, NULL));
- free_states(env);
-
- mutex_unlock(&bpf_verifier_lock);
- vfree(env->insn_aux_data);
-err_free_env:
- kfree(env);
- return ret;
-}
-EXPORT_SYMBOL_GPL(bpf_analyzer);
diff --git a/net/core/filter.c b/net/core/filter.c
index a0112168d6f9..1afa17935954 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3777,25 +3777,6 @@ static bool tc_cls_act_is_valid_access(int off, int size,
return bpf_skb_is_valid_access(off, size, type, info);
}
-static bool
-tc_cls_act_is_valid_access_analyzer(int off, int size,
- enum bpf_access_type type,
- struct bpf_insn_access_aux *info)
-{
- switch (off) {
- case offsetof(struct sk_buff, len):
- return true;
- case offsetof(struct sk_buff, data):
- info->reg_type = PTR_TO_PACKET;
- return true;
- case offsetof(struct sk_buff, cb) +
- offsetof(struct bpf_skb_data_end, data_end):
- info->reg_type = PTR_TO_PACKET_END;
- return true;
- }
- return false;
-}
-
static bool __is_valid_xdp_access(int off, int size)
{
if (off < 0 || off >= sizeof(struct xdp_md))
@@ -3830,21 +3811,6 @@ static bool xdp_is_valid_access(int off, int size,
return __is_valid_xdp_access(off, size);
}
-static bool xdp_is_valid_access_analyzer(int off, int size,
- enum bpf_access_type type,
- struct bpf_insn_access_aux *info)
-{
- switch (off) {
- case offsetof(struct xdp_buff, data):
- info->reg_type = PTR_TO_PACKET;
- return true;
- case offsetof(struct xdp_buff, data_end):
- info->reg_type = PTR_TO_PACKET_END;
- return true;
- }
- return false;
-}
-
void bpf_warn_invalid_xdp_action(u32 act)
{
const u32 act_max = XDP_REDIRECT;
@@ -4516,10 +4482,6 @@ const struct bpf_verifier_ops tc_cls_act_verifier_ops = {
.gen_prologue = tc_cls_act_prologue,
};
-const struct bpf_verifier_ops tc_cls_act_analyzer_ops = {
- .is_valid_access = tc_cls_act_is_valid_access_analyzer,
-};
-
const struct bpf_prog_ops tc_cls_act_prog_ops = {
.test_run = bpf_prog_test_run_skb,
};
@@ -4530,10 +4492,6 @@ const struct bpf_verifier_ops xdp_verifier_ops = {
.convert_ctx_access = xdp_convert_ctx_access,
};
-const struct bpf_verifier_ops xdp_analyzer_ops = {
- .is_valid_access = xdp_is_valid_access_analyzer,
-};
-
const struct bpf_prog_ops xdp_prog_ops = {
.test_run = bpf_prog_test_run_xdp,
};
--
2.14.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH net-next v2 00/15] bpf: add offload as a first class citizen
2017-11-03 20:56 [PATCH net-next v2 00/15] bpf: add offload as a first class citizen Jakub Kicinski
` (14 preceding siblings ...)
2017-11-03 20:56 ` [PATCH net-next v2 15/15] bpf: remove old offload/analyzer Jakub Kicinski
@ 2017-11-05 13:28 ` David Miller
15 siblings, 0 replies; 23+ messages in thread
From: David Miller @ 2017-11-05 13:28 UTC (permalink / raw)
To: jakub.kicinski; +Cc: netdev, oss-drivers, alexei.starovoitov, daniel
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Fri, 3 Nov 2017 13:56:15 -0700
> This series is my stab at what was discussed at a recent IOvisor
> bi-weekly call. The idea is to make the device translator run at
> the program load time. This makes the offload more explicit to
> the user space. It also makes it easy for the device translator
> to insert information into the original verifier log.
>
> v2:
> - include linux/bug.h instead of asm/bug.h;
> - rebased on top of Craig's verifier fix (no changes, the last patch
> just removes more code now). I checked the set doesn't conflict
> with Jiri's, Josef's or Roman's patches, but missed Craig's fix :(
> v1:
> - rename the ifindex member on load;
> - improve commit messages;
> - split nfp patches more.
Series applied, thanks Jakub.
^ permalink raw reply [flat|nested] 23+ messages in thread