* [PATCH net-next 1/4] netlink: add NULL-friendly helper for setting extended ACK message
2017-05-01 4:46 [PATCH net-next 0/4] xdp: use netlink extended ACK reporting Jakub Kicinski
@ 2017-05-01 4:46 ` Jakub Kicinski
2017-05-01 10:45 ` Daniel Borkmann
2017-05-01 4:46 ` [PATCH net-next 2/4] xdp: propagate extended ack to XDP setup Jakub Kicinski
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2017-05-01 4:46 UTC (permalink / raw)
To: netdev
Cc: davem, johannes, dsa, daniel, alexei.starovoitov, bblanco,
john.fastabend, kubakici, oss-drivers, brouer, jhs,
Jakub Kicinski
As we propagate extended ack reporting throughout various paths in
the kernel it may be that the same function is called with the
extended ack parameter passed as NULL. One place where that happens
is in drivers which have a centralized reconfiguration function
called both from ndos and from ethtool_ops. Add a new helper for
setting the error message in such conditions.
Existing helper is left as is to encourage propagating the ext act
fully wherever possible. It also makes it clear in the code which
messages may be lost due to ext ack being NULL.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
include/linux/netlink.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index 8d2a8924705c..c20395edf2de 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -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);
--
2.11.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH net-next 1/4] netlink: add NULL-friendly helper for setting extended ACK message
2017-05-01 4:46 ` [PATCH net-next 1/4] netlink: add NULL-friendly helper for setting extended ACK message Jakub Kicinski
@ 2017-05-01 10:45 ` Daniel Borkmann
0 siblings, 0 replies; 12+ messages in thread
From: Daniel Borkmann @ 2017-05-01 10:45 UTC (permalink / raw)
To: Jakub Kicinski, netdev
Cc: davem, johannes, dsa, alexei.starovoitov, bblanco, john.fastabend,
kubakici, oss-drivers, brouer, jhs
On 05/01/2017 06:46 AM, Jakub Kicinski wrote:
> As we propagate extended ack reporting throughout various paths in
> the kernel it may be that the same function is called with the
> extended ack parameter passed as NULL. One place where that happens
> is in drivers which have a centralized reconfiguration function
> called both from ndos and from ethtool_ops. Add a new helper for
> setting the error message in such conditions.
>
> Existing helper is left as is to encourage propagating the ext act
> fully wherever possible. It also makes it clear in the code which
> messages may be lost due to ext ack being NULL.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net-next 2/4] xdp: propagate extended ack to XDP setup
2017-05-01 4:46 [PATCH net-next 0/4] xdp: use netlink extended ACK reporting Jakub Kicinski
2017-05-01 4:46 ` [PATCH net-next 1/4] netlink: add NULL-friendly helper for setting extended ACK message Jakub Kicinski
@ 2017-05-01 4:46 ` Jakub Kicinski
2017-05-01 10:46 ` Daniel Borkmann
2017-05-01 4:46 ` [PATCH net-next 3/4] nfp: make use of extended ack message reporting Jakub Kicinski
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2017-05-01 4:46 UTC (permalink / raw)
To: netdev
Cc: davem, johannes, dsa, daniel, alexei.starovoitov, bblanco,
john.fastabend, kubakici, oss-drivers, brouer, jhs,
Jakub Kicinski
Drivers usually have a number of restrictions for running XDP
- most common being buffer sizes, LRO and number of rings.
Even though some drivers try to be helpful and print error
messages experience shows that users don't often consult
kernel logs on netlink errors. Try to use the new extended
ack mechanism to carry the message back to user space.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
include/linux/netdevice.h | 10 ++++++++--
net/core/dev.c | 5 ++++-
net/core/rtnetlink.c | 13 ++++++++-----
3 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 6847714a5ae3..9c23bd2efb56 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -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;
};
@@ -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);
diff --git a/net/core/dev.c b/net/core/dev.c
index 8371a01eee87..35a06cebb282 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -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;
@@ -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);
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 9031a6c8bfa7..6e67315ec368 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -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;
@@ -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)
@@ -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;
}
@@ -2423,6 +2424,7 @@ 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;
@@ -2430,7 +2432,7 @@ static int rtnl_group_changelink(const struct sk_buff *skb,
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;
}
@@ -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;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH net-next 2/4] xdp: propagate extended ack to XDP setup
2017-05-01 4:46 ` [PATCH net-next 2/4] xdp: propagate extended ack to XDP setup Jakub Kicinski
@ 2017-05-01 10:46 ` Daniel Borkmann
0 siblings, 0 replies; 12+ messages in thread
From: Daniel Borkmann @ 2017-05-01 10:46 UTC (permalink / raw)
To: Jakub Kicinski, netdev
Cc: davem, johannes, dsa, alexei.starovoitov, bblanco, john.fastabend,
kubakici, oss-drivers, brouer, jhs
On 05/01/2017 06:46 AM, Jakub Kicinski wrote:
> Drivers usually have a number of restrictions for running XDP
> - most common being buffer sizes, LRO and number of rings.
> Even though some drivers try to be helpful and print error
> messages experience shows that users don't often consult
> kernel logs on netlink errors. Try to use the new extended
> ack mechanism to carry the message back to user space.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net-next 3/4] nfp: make use of extended ack message reporting
2017-05-01 4:46 [PATCH net-next 0/4] xdp: use netlink extended ACK reporting Jakub Kicinski
2017-05-01 4:46 ` [PATCH net-next 1/4] netlink: add NULL-friendly helper for setting extended ACK message Jakub Kicinski
2017-05-01 4:46 ` [PATCH net-next 2/4] xdp: propagate extended ack to XDP setup Jakub Kicinski
@ 2017-05-01 4:46 ` Jakub Kicinski
2017-05-01 10:46 ` Daniel Borkmann
2017-05-01 4:46 ` [PATCH net-next 4/4] virtio_net: " Jakub Kicinski
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2017-05-01 4:46 UTC (permalink / raw)
To: netdev
Cc: davem, johannes, dsa, daniel, alexei.starovoitov, bblanco,
john.fastabend, kubakici, oss-drivers, brouer, jhs,
Jakub Kicinski
Try to carry error messages to the user via the netlink extended
ack message attribute.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
drivers/net/ethernet/netronome/nfp/nfp_net.h | 3 ++-
.../net/ethernet/netronome/nfp/nfp_net_common.c | 22 +++++++++++++---------
.../net/ethernet/netronome/nfp/nfp_net_ethtool.c | 4 ++--
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net.h b/drivers/net/ethernet/netronome/nfp/nfp_net.h
index 38b41fdeaa8f..fcf81b3be830 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net.h
@@ -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);
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index b9f3548bb65f..db20376260f5 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -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;
@@ -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;
@@ -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,
@@ -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;
@@ -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;
@@ -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;
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
index a704efd4e314..abbb47e60cc3 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
@@ -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,
@@ -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,
--
2.11.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH net-next 3/4] nfp: make use of extended ack message reporting
2017-05-01 4:46 ` [PATCH net-next 3/4] nfp: make use of extended ack message reporting Jakub Kicinski
@ 2017-05-01 10:46 ` Daniel Borkmann
0 siblings, 0 replies; 12+ messages in thread
From: Daniel Borkmann @ 2017-05-01 10:46 UTC (permalink / raw)
To: Jakub Kicinski, netdev
Cc: davem, johannes, dsa, alexei.starovoitov, bblanco, john.fastabend,
kubakici, oss-drivers, brouer, jhs
On 05/01/2017 06:46 AM, Jakub Kicinski wrote:
> Try to carry error messages to the user via the netlink extended
> ack message attribute.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net-next 4/4] virtio_net: make use of extended ack message reporting
2017-05-01 4:46 [PATCH net-next 0/4] xdp: use netlink extended ACK reporting Jakub Kicinski
` (2 preceding siblings ...)
2017-05-01 4:46 ` [PATCH net-next 3/4] nfp: make use of extended ack message reporting Jakub Kicinski
@ 2017-05-01 4:46 ` Jakub Kicinski
2017-05-01 10:50 ` Daniel Borkmann
2017-05-01 10:32 ` [oss-drivers] [PATCH net-next 0/4] xdp: use netlink extended ACK reporting Simon Horman
2017-05-01 14:36 ` David Miller
5 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2017-05-01 4:46 UTC (permalink / raw)
To: netdev
Cc: davem, johannes, dsa, daniel, alexei.starovoitov, bblanco,
john.fastabend, kubakici, oss-drivers, brouer, jhs,
Jakub Kicinski
Try to carry error messages to the user via the netlink extended
ack message attribute.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
drivers/net/virtio_net.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 82f1c3a73345..046c60619c59 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -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);
@@ -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;
}
@@ -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;
@@ -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;
--
2.11.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH net-next 4/4] virtio_net: make use of extended ack message reporting
2017-05-01 4:46 ` [PATCH net-next 4/4] virtio_net: " Jakub Kicinski
@ 2017-05-01 10:50 ` Daniel Borkmann
2017-05-01 14:34 ` David Miller
0 siblings, 1 reply; 12+ messages in thread
From: Daniel Borkmann @ 2017-05-01 10:50 UTC (permalink / raw)
To: Jakub Kicinski, netdev
Cc: davem, johannes, dsa, alexei.starovoitov, bblanco, john.fastabend,
kubakici, oss-drivers, brouer, jhs
On 05/01/2017 06:46 AM, Jakub Kicinski wrote:
> Try to carry error messages to the user via the netlink extended
> ack message attribute.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
[...]
> @@ -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");
Should this be NL_MOD_TRY_SET_ERR_MSG() as well like in nfp case
(otherwise the 'if (_extack)' check might be missing from the
macro)?
Thanks,
Daniel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next 4/4] virtio_net: make use of extended ack message reporting
2017-05-01 10:50 ` Daniel Borkmann
@ 2017-05-01 14:34 ` David Miller
0 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2017-05-01 14:34 UTC (permalink / raw)
To: daniel
Cc: jakub.kicinski, netdev, johannes, dsa, alexei.starovoitov,
bblanco, john.fastabend, kubakici, oss-drivers, brouer, jhs
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Mon, 01 May 2017 12:50:55 +0200
> On 05/01/2017 06:46 AM, Jakub Kicinski wrote:
>> Try to carry error messages to the user via the netlink extended
>> ack message attribute.
>>
>> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>
> [...]
>> @@ -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");
>
> Should this be NL_MOD_TRY_SET_ERR_MSG() as well like in nfp case
> (otherwise the 'if (_extack)' check might be missing from the
> macro)?
I don't see a path in the doit callchain for setlink/newlink where the
extack pointer can be null.
I'm going to apply this series and we should either:
1) Guarantee the extack object is always available.
or
2) Make all macros including NL_SET_ERR_MSG() check the
pointer.
Thanks.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [oss-drivers] [PATCH net-next 0/4] xdp: use netlink extended ACK reporting
2017-05-01 4:46 [PATCH net-next 0/4] xdp: use netlink extended ACK reporting Jakub Kicinski
` (3 preceding siblings ...)
2017-05-01 4:46 ` [PATCH net-next 4/4] virtio_net: " Jakub Kicinski
@ 2017-05-01 10:32 ` Simon Horman
2017-05-01 14:36 ` David Miller
5 siblings, 0 replies; 12+ messages in thread
From: Simon Horman @ 2017-05-01 10:32 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, davem, johannes, dsa, daniel, alexei.starovoitov, bblanco,
john.fastabend, kubakici, oss-drivers, brouer, jhs
On Sun, Apr 30, 2017 at 09:46:44PM -0700, Jakub Kicinski wrote:
> Hi!
>
> 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:
>
> # ip link set dev p4p1 xdp obj ipip_prepend.o sec ".text"
> 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.
>
> Jakub Kicinski (4):
> netlink: add NULL-friendly helper for setting extended ACK message
> xdp: propagate extended ack to XDP setup
> nfp: make use of extended ack message reporting
> virtio_net: make use of extended ack message reporting
Reviewed-by: Simon Horman <simon.horman@netronome.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next 0/4] xdp: use netlink extended ACK reporting
2017-05-01 4:46 [PATCH net-next 0/4] xdp: use netlink extended ACK reporting Jakub Kicinski
` (4 preceding siblings ...)
2017-05-01 10:32 ` [oss-drivers] [PATCH net-next 0/4] xdp: use netlink extended ACK reporting Simon Horman
@ 2017-05-01 14:36 ` David Miller
5 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2017-05-01 14:36 UTC (permalink / raw)
To: jakub.kicinski
Cc: netdev, johannes, dsa, daniel, alexei.starovoitov, bblanco,
john.fastabend, kubakici, oss-drivers, brouer, jhs
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Sun, 30 Apr 2017 21:46:44 -0700
> 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:
>
> # ip link set dev p4p1 xdp obj ipip_prepend.o sec ".text"
> 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.
Series applied.
^ permalink raw reply [flat|nested] 12+ messages in thread