Skip to content

Commit

Permalink
Merge branch 'xdp-netlink-ext-ack'
Browse files Browse the repository at this point in the history
Jakub Kicinski says:

====================
xdp: use netlink extended ACK reporting

This series is an attempt to make XDP more user friendly by
enabling exploiting the recently added netlink extended ACK
reporting to carry messages to user space.

David Ahern's iproute2 ext ack patches for ip link are sufficient
to show the errors like this:

Error: nfp: MTU too large w/ XDP enabled

Where the message is coming directly from the driver.  There could
still be a bit of a leap for a complete novice from the message
above to the right settings, but it's a big improvement over the
standard "Invalid argument" message.

v1/non-rfc:
 - add a separate macro in patch 1;
 - add KBUILD_MODNAME as part of the message (Daniel);
 - don't print the error to logs in patch 1.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed May 1, 2017
2 parents 7333362 + 9861ce0 commit d74a32a
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 24 deletions.
3 changes: 2 additions & 1 deletion drivers/net/ethernet/netronome/nfp/nfp_net.h
Original file line number Diff line number Diff line change
Expand Up @@ -818,7 +818,8 @@ nfp_net_irqs_assign(struct nfp_net *nn, struct msix_entry *irq_entries,
unsigned int n);

struct nfp_net_dp *nfp_net_clone_dp(struct nfp_net *nn);
int nfp_net_ring_reconfig(struct nfp_net *nn, struct nfp_net_dp *new);
int nfp_net_ring_reconfig(struct nfp_net *nn, struct nfp_net_dp *new,
struct netlink_ext_ack *extack);

bool nfp_net_link_changed_read_clear(struct nfp_net *nn);
int nfp_net_refresh_eth_port(struct nfp_net *nn);
Expand Down
22 changes: 13 additions & 9 deletions drivers/net/ethernet/netronome/nfp/nfp_net_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -2524,24 +2524,27 @@ struct nfp_net_dp *nfp_net_clone_dp(struct nfp_net *nn)
return new;
}

static int nfp_net_check_config(struct nfp_net *nn, struct nfp_net_dp *dp)
static int
nfp_net_check_config(struct nfp_net *nn, struct nfp_net_dp *dp,
struct netlink_ext_ack *extack)
{
/* XDP-enabled tests */
if (!dp->xdp_prog)
return 0;
if (dp->fl_bufsz > PAGE_SIZE) {
nn_warn(nn, "MTU too large w/ XDP enabled\n");
NL_MOD_TRY_SET_ERR_MSG(extack, "MTU too large w/ XDP enabled");
return -EINVAL;
}
if (dp->num_tx_rings > nn->max_tx_rings) {
nn_warn(nn, "Insufficient number of TX rings w/ XDP enabled\n");
NL_MOD_TRY_SET_ERR_MSG(extack, "Insufficient number of TX rings w/ XDP enabled");
return -EINVAL;
}

return 0;
}

int nfp_net_ring_reconfig(struct nfp_net *nn, struct nfp_net_dp *dp)
int nfp_net_ring_reconfig(struct nfp_net *nn, struct nfp_net_dp *dp,
struct netlink_ext_ack *extack)
{
int r, err;

Expand All @@ -2553,7 +2556,7 @@ int nfp_net_ring_reconfig(struct nfp_net *nn, struct nfp_net_dp *dp)

dp->num_r_vecs = max(dp->num_rx_rings, dp->num_stack_tx_rings);

err = nfp_net_check_config(nn, dp);
err = nfp_net_check_config(nn, dp, extack);
if (err)
goto exit_free_dp;

Expand Down Expand Up @@ -2628,7 +2631,7 @@ static int nfp_net_change_mtu(struct net_device *netdev, int new_mtu)

dp->mtu = new_mtu;

return nfp_net_ring_reconfig(nn, dp);
return nfp_net_ring_reconfig(nn, dp, NULL);
}

static void nfp_net_stat64(struct net_device *netdev,
Expand Down Expand Up @@ -2944,9 +2947,10 @@ static int nfp_net_xdp_offload(struct nfp_net *nn, struct bpf_prog *prog)
return ret;
}

static int nfp_net_xdp_setup(struct nfp_net *nn, struct bpf_prog *prog)
static int nfp_net_xdp_setup(struct nfp_net *nn, struct netdev_xdp *xdp)
{
struct bpf_prog *old_prog = nn->dp.xdp_prog;
struct bpf_prog *prog = xdp->prog;
struct nfp_net_dp *dp;
int err;

Expand All @@ -2969,7 +2973,7 @@ static int nfp_net_xdp_setup(struct nfp_net *nn, struct bpf_prog *prog)
dp->rx_dma_off = prog ? XDP_PACKET_HEADROOM - nn->dp.rx_offset : 0;

/* We need RX reconfig to remap the buffers (BIDIR vs FROM_DEV) */
err = nfp_net_ring_reconfig(nn, dp);
err = nfp_net_ring_reconfig(nn, dp, xdp->extack);
if (err)
return err;

Expand All @@ -2987,7 +2991,7 @@ static int nfp_net_xdp(struct net_device *netdev, struct netdev_xdp *xdp)

switch (xdp->command) {
case XDP_SETUP_PROG:
return nfp_net_xdp_setup(nn, xdp->prog);
return nfp_net_xdp_setup(nn, xdp);
case XDP_QUERY_PROG:
xdp->prog_attached = !!nn->dp.xdp_prog;
return 0;
Expand Down
4 changes: 2 additions & 2 deletions drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ static int nfp_net_set_ring_size(struct nfp_net *nn, u32 rxd_cnt, u32 txd_cnt)
dp->rxd_cnt = rxd_cnt;
dp->txd_cnt = txd_cnt;

return nfp_net_ring_reconfig(nn, dp);
return nfp_net_ring_reconfig(nn, dp, NULL);
}

static int nfp_net_set_ringparam(struct net_device *netdev,
Expand Down Expand Up @@ -880,7 +880,7 @@ static int nfp_net_set_num_rings(struct nfp_net *nn, unsigned int total_rx,
if (dp->xdp_prog)
dp->num_tx_rings += total_rx;

return nfp_net_ring_reconfig(nn, dp);
return nfp_net_ring_reconfig(nn, dp, NULL);
}

static int nfp_net_set_channels(struct net_device *netdev,
Expand Down
11 changes: 7 additions & 4 deletions drivers/net/virtio_net.c
Original file line number Diff line number Diff line change
Expand Up @@ -1878,7 +1878,8 @@ static int virtnet_reset(struct virtnet_info *vi, int curr_qp, int xdp_qp)
return ret;
}

static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog)
static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
struct netlink_ext_ack *extack)
{
unsigned long int max_sz = PAGE_SIZE - sizeof(struct padded_vnet_hdr);
struct virtnet_info *vi = netdev_priv(dev);
Expand All @@ -1890,16 +1891,17 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog)
virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO6) ||
virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_ECN) ||
virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_UFO)) {
netdev_warn(dev, "can't set XDP while host is implementing LRO, disable LRO first\n");
NL_SET_ERR_MSG(extack, "can't set XDP while host is implementing LRO, disable LRO first");
return -EOPNOTSUPP;
}

if (vi->mergeable_rx_bufs && !vi->any_header_sg) {
netdev_warn(dev, "XDP expects header/data in single page, any_header_sg required\n");
NL_SET_ERR_MSG(extack, "XDP expects header/data in single page, any_header_sg required");
return -EINVAL;
}

if (dev->mtu > max_sz) {
NL_SET_ERR_MSG(extack, "MTU too large to enable XDP");
netdev_warn(dev, "XDP requires MTU less than %lu\n", max_sz);
return -EINVAL;
}
Expand All @@ -1910,6 +1912,7 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog)

/* XDP requires extra queues for XDP_TX */
if (curr_qp + xdp_qp > vi->max_queue_pairs) {
NL_SET_ERR_MSG(extack, "Too few free TX rings available");
netdev_warn(dev, "request %i queues but max is %i\n",
curr_qp + xdp_qp, vi->max_queue_pairs);
return -ENOMEM;
Expand Down Expand Up @@ -1971,7 +1974,7 @@ static int virtnet_xdp(struct net_device *dev, struct netdev_xdp *xdp)
{
switch (xdp->command) {
case XDP_SETUP_PROG:
return virtnet_xdp_set(dev, xdp->prog);
return virtnet_xdp_set(dev, xdp->prog, xdp->extack);
case XDP_QUERY_PROG:
xdp->prog_attached = virtnet_xdp_query(dev);
return 0;
Expand Down
10 changes: 8 additions & 2 deletions include/linux/netdevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -813,11 +813,16 @@ enum xdp_netdev_command {
XDP_QUERY_PROG,
};

struct netlink_ext_ack;

struct netdev_xdp {
enum xdp_netdev_command command;
union {
/* XDP_SETUP_PROG */
struct bpf_prog *prog;
struct {
struct bpf_prog *prog;
struct netlink_ext_ack *extack;
};
/* XDP_QUERY_PROG */
bool prog_attached;
};
Expand Down Expand Up @@ -3291,7 +3296,8 @@ int dev_get_phys_port_id(struct net_device *dev,
int dev_get_phys_port_name(struct net_device *dev,
char *name, size_t len);
int dev_change_proto_down(struct net_device *dev, bool proto_down);
int dev_change_xdp_fd(struct net_device *dev, int fd, u32 flags);
int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
int fd, u32 flags);
struct sk_buff *validate_xmit_skb_list(struct sk_buff *skb, struct net_device *dev);
struct sk_buff *dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
struct netdev_queue *txq, int *ret);
Expand Down
8 changes: 8 additions & 0 deletions include/linux/netlink.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,14 @@ struct netlink_ext_ack {
(extack)->_msg = _msg; \
} while (0)

#define NL_MOD_TRY_SET_ERR_MSG(extack, msg) do { \
static const char _msg[] = KBUILD_MODNAME ": " msg; \
struct netlink_ext_ack *_extack = (extack); \
\
if (_extack) \
_extack->_msg = _msg; \
} while (0)

extern void netlink_kernel_release(struct sock *sk);
extern int __netlink_change_ngroups(struct sock *sk, unsigned int groups);
extern int netlink_change_ngroups(struct sock *sk, unsigned int groups);
Expand Down
5 changes: 4 additions & 1 deletion net/core/dev.c
Original file line number Diff line number Diff line change
Expand Up @@ -6854,12 +6854,14 @@ EXPORT_SYMBOL(dev_change_proto_down);
/**
* dev_change_xdp_fd - set or clear a bpf program for a device rx path
* @dev: device
* @extact: netlink extended ack
* @fd: new program fd or negative value to clear
* @flags: xdp-related flags
*
* Set or clear a bpf program for a device
*/
int dev_change_xdp_fd(struct net_device *dev, int fd, u32 flags)
int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
int fd, u32 flags)
{
int (*xdp_op)(struct net_device *dev, struct netdev_xdp *xdp);
const struct net_device_ops *ops = dev->netdev_ops;
Expand Down Expand Up @@ -6892,6 +6894,7 @@ int dev_change_xdp_fd(struct net_device *dev, int fd, u32 flags)

memset(&xdp, 0, sizeof(xdp));
xdp.command = XDP_SETUP_PROG;
xdp.extack = extack;
xdp.prog = prog;

err = xdp_op(dev, &xdp);
Expand Down
13 changes: 8 additions & 5 deletions net/core/rtnetlink.c
Original file line number Diff line number Diff line change
Expand Up @@ -1919,6 +1919,7 @@ static int do_set_master(struct net_device *dev, int ifindex)
#define DO_SETLINK_NOTIFY 0x03
static int do_setlink(const struct sk_buff *skb,
struct net_device *dev, struct ifinfomsg *ifm,
struct netlink_ext_ack *extack,
struct nlattr **tb, char *ifname, int status)
{
const struct net_device_ops *ops = dev->netdev_ops;
Expand Down Expand Up @@ -2201,7 +2202,7 @@ static int do_setlink(const struct sk_buff *skb,
}

if (xdp[IFLA_XDP_FD]) {
err = dev_change_xdp_fd(dev,
err = dev_change_xdp_fd(dev, extack,
nla_get_s32(xdp[IFLA_XDP_FD]),
xdp_flags);
if (err)
Expand Down Expand Up @@ -2261,7 +2262,7 @@ static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh,
if (err < 0)
goto errout;

err = do_setlink(skb, dev, ifm, tb, ifname, 0);
err = do_setlink(skb, dev, ifm, extack, tb, ifname, 0);
errout:
return err;
}
Expand Down Expand Up @@ -2423,14 +2424,15 @@ EXPORT_SYMBOL(rtnl_create_link);
static int rtnl_group_changelink(const struct sk_buff *skb,
struct net *net, int group,
struct ifinfomsg *ifm,
struct netlink_ext_ack *extack,
struct nlattr **tb)
{
struct net_device *dev, *aux;
int err;

for_each_netdev_safe(net, dev, aux) {
if (dev->group == group) {
err = do_setlink(skb, dev, ifm, tb, NULL, 0);
err = do_setlink(skb, dev, ifm, extack, tb, NULL, 0);
if (err < 0)
return err;
}
Expand Down Expand Up @@ -2576,14 +2578,15 @@ static int rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
status |= DO_SETLINK_NOTIFY;
}

return do_setlink(skb, dev, ifm, tb, ifname, status);
return do_setlink(skb, dev, ifm, extack, tb, ifname,
status);
}

if (!(nlh->nlmsg_flags & NLM_F_CREATE)) {
if (ifm->ifi_index == 0 && tb[IFLA_GROUP])
return rtnl_group_changelink(skb, net,
nla_get_u32(tb[IFLA_GROUP]),
ifm, tb);
ifm, extack, tb);
return -ENODEV;
}

Expand Down

0 comments on commit d74a32a

Please sign in to comment.