* [Intel-wired-lan] [PATCH net v3 0/6] igb: xdp patches followup
@ 2020-10-19 8:05 ` sven.auhagen
0 siblings, 0 replies; 28+ messages in thread
From: sven.auhagen @ 2020-10-19 8:05 UTC (permalink / raw)
To: intel-wired-lan
From: Sven Auhagen <sven.auhagen@voleatech.de>
This patch series addresses some of the comments that came back
after the igb XDP patch was accepted.
Most of it is code cleanup.
The last patch contains a fix for a tx queue timeout
that can occur when using xdp.
Change from v2:
* Move SOB line to end
* Remove SOB from cover letter
* Add a better commit message for
avoid transmit queue timeout in xdp path
* Add ACK by from Maciej Fijalkowski
Change from v1:
* Drop patch 5 as the igb_rx_frame_truesize won't match
* Fix typo in comment
* Add Suggested-by and Reviewed-by tags
* Add how to avoid transmit queue timeout in xdp path
is fixed in the commit message
Sven Auhagen (6):
igb: XDP xmit back fix error code
igb: take vlan double header into account
igb: XDP extack message on error
igb: skb add metasize for xdp
igb: use xdp_do_flush
igb: avoid transmit queue timeout in xdp path
drivers/net/ethernet/intel/igb/igb.h | 5 ++++
drivers/net/ethernet/intel/igb/igb_main.c | 32 +++++++++++++++--------
2 files changed, 26 insertions(+), 11 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 28+ messages in thread* [PATCH net v3 1/6] igb: XDP xmit back fix error code
2020-10-19 8:05 ` [Intel-wired-lan] " sven.auhagen
@ 2020-10-19 8:05 ` sven.auhagen
-1 siblings, 0 replies; 28+ messages in thread
From: sven.auhagen @ 2020-10-19 8:05 UTC (permalink / raw)
To: anthony.l.nguyen, maciej.fijalkowski, kuba
Cc: davem, intel-wired-lan, netdev, nhorman, sassmann,
sandeep.penigalapati, brouer
From: Sven Auhagen <sven.auhagen@voleatech.de>
The igb XDP xmit back function should only return
defined error codes.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
---
drivers/net/ethernet/intel/igb/igb_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 5fc2c381da55..08cc6f59aa2e 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2910,7 +2910,7 @@ static int igb_xdp_xmit_back(struct igb_adapter *adapter, struct xdp_buff *xdp)
*/
tx_ring = adapter->xdp_prog ? igb_xdp_tx_queue_mapping(adapter) : NULL;
if (unlikely(!tx_ring))
- return -ENXIO;
+ return IGB_XDP_CONSUMED;
nq = txring_txq(tx_ring);
__netif_tx_lock(nq, cpu);
--
2.20.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Intel-wired-lan] [PATCH net v3 1/6] igb: XDP xmit back fix error code
@ 2020-10-19 8:05 ` sven.auhagen
0 siblings, 0 replies; 28+ messages in thread
From: sven.auhagen @ 2020-10-19 8:05 UTC (permalink / raw)
To: intel-wired-lan
From: Sven Auhagen <sven.auhagen@voleatech.de>
The igb XDP xmit back function should only return
defined error codes.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
---
drivers/net/ethernet/intel/igb/igb_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 5fc2c381da55..08cc6f59aa2e 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2910,7 +2910,7 @@ static int igb_xdp_xmit_back(struct igb_adapter *adapter, struct xdp_buff *xdp)
*/
tx_ring = adapter->xdp_prog ? igb_xdp_tx_queue_mapping(adapter) : NULL;
if (unlikely(!tx_ring))
- return -ENXIO;
+ return IGB_XDP_CONSUMED;
nq = txring_txq(tx_ring);
__netif_tx_lock(nq, cpu);
--
2.20.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH net v3 2/6] igb: take vlan double header into account
2020-10-19 8:05 ` [Intel-wired-lan] " sven.auhagen
@ 2020-10-19 8:05 ` sven.auhagen
-1 siblings, 0 replies; 28+ messages in thread
From: sven.auhagen @ 2020-10-19 8:05 UTC (permalink / raw)
To: anthony.l.nguyen, maciej.fijalkowski, kuba
Cc: davem, intel-wired-lan, netdev, nhorman, sassmann,
sandeep.penigalapati, brouer
From: Sven Auhagen <sven.auhagen@voleatech.de>
Increase the packet header padding to include double VLAN tagging.
This patch uses a macro for this.
Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
---
drivers/net/ethernet/intel/igb/igb.h | 5 +++++
drivers/net/ethernet/intel/igb/igb_main.c | 7 +++----
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 0286d2fceee4..aaa954aae574 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -138,6 +138,8 @@ struct vf_mac_filter {
/* this is the size past which hardware will drop packets when setting LPE=0 */
#define MAXIMUM_ETHERNET_VLAN_SIZE 1522
+#define IGB_ETH_PKT_HDR_PAD (ETH_HLEN + ETH_FCS_LEN + (VLAN_HLEN * 2))
+
/* Supported Rx Buffer Sizes */
#define IGB_RXBUFFER_256 256
#define IGB_RXBUFFER_1536 1536
@@ -247,6 +249,9 @@ enum igb_tx_flags {
#define IGB_SFF_ADDRESSING_MODE 0x4
#define IGB_SFF_8472_UNSUP 0x00
+/* TX resources are shared between XDP and netstack
+ * and we need to tag the buffer type to distinguish them
+ */
enum igb_tx_buf_type {
IGB_TYPE_SKB = 0,
IGB_TYPE_XDP,
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 08cc6f59aa2e..0a9198037b98 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2826,7 +2826,7 @@ static int igb_setup_tc(struct net_device *dev, enum tc_setup_type type,
static int igb_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
{
- int i, frame_size = dev->mtu + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN;
+ int i, frame_size = dev->mtu + IGB_ETH_PKT_HDR_PAD;
struct igb_adapter *adapter = netdev_priv(dev);
bool running = netif_running(dev);
struct bpf_prog *old_prog;
@@ -3950,8 +3950,7 @@ static int igb_sw_init(struct igb_adapter *adapter)
/* set default work limits */
adapter->tx_work_limit = IGB_DEFAULT_TX_WORK;
- adapter->max_frame_size = netdev->mtu + ETH_HLEN + ETH_FCS_LEN +
- VLAN_HLEN;
+ adapter->max_frame_size = netdev->mtu + IGB_ETH_PKT_HDR_PAD;
adapter->min_frame_size = ETH_ZLEN + ETH_FCS_LEN;
spin_lock_init(&adapter->nfc_lock);
@@ -6491,7 +6490,7 @@ static void igb_get_stats64(struct net_device *netdev,
static int igb_change_mtu(struct net_device *netdev, int new_mtu)
{
struct igb_adapter *adapter = netdev_priv(netdev);
- int max_frame = new_mtu + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN;
+ int max_frame = new_mtu + IGB_ETH_PKT_HDR_PAD;
if (adapter->xdp_prog) {
int i;
--
2.20.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Intel-wired-lan] [PATCH net v3 2/6] igb: take vlan double header into account
@ 2020-10-19 8:05 ` sven.auhagen
0 siblings, 0 replies; 28+ messages in thread
From: sven.auhagen @ 2020-10-19 8:05 UTC (permalink / raw)
To: intel-wired-lan
From: Sven Auhagen <sven.auhagen@voleatech.de>
Increase the packet header padding to include double VLAN tagging.
This patch uses a macro for this.
Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
---
drivers/net/ethernet/intel/igb/igb.h | 5 +++++
drivers/net/ethernet/intel/igb/igb_main.c | 7 +++----
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 0286d2fceee4..aaa954aae574 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -138,6 +138,8 @@ struct vf_mac_filter {
/* this is the size past which hardware will drop packets when setting LPE=0 */
#define MAXIMUM_ETHERNET_VLAN_SIZE 1522
+#define IGB_ETH_PKT_HDR_PAD (ETH_HLEN + ETH_FCS_LEN + (VLAN_HLEN * 2))
+
/* Supported Rx Buffer Sizes */
#define IGB_RXBUFFER_256 256
#define IGB_RXBUFFER_1536 1536
@@ -247,6 +249,9 @@ enum igb_tx_flags {
#define IGB_SFF_ADDRESSING_MODE 0x4
#define IGB_SFF_8472_UNSUP 0x00
+/* TX resources are shared between XDP and netstack
+ * and we need to tag the buffer type to distinguish them
+ */
enum igb_tx_buf_type {
IGB_TYPE_SKB = 0,
IGB_TYPE_XDP,
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 08cc6f59aa2e..0a9198037b98 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2826,7 +2826,7 @@ static int igb_setup_tc(struct net_device *dev, enum tc_setup_type type,
static int igb_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
{
- int i, frame_size = dev->mtu + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN;
+ int i, frame_size = dev->mtu + IGB_ETH_PKT_HDR_PAD;
struct igb_adapter *adapter = netdev_priv(dev);
bool running = netif_running(dev);
struct bpf_prog *old_prog;
@@ -3950,8 +3950,7 @@ static int igb_sw_init(struct igb_adapter *adapter)
/* set default work limits */
adapter->tx_work_limit = IGB_DEFAULT_TX_WORK;
- adapter->max_frame_size = netdev->mtu + ETH_HLEN + ETH_FCS_LEN +
- VLAN_HLEN;
+ adapter->max_frame_size = netdev->mtu + IGB_ETH_PKT_HDR_PAD;
adapter->min_frame_size = ETH_ZLEN + ETH_FCS_LEN;
spin_lock_init(&adapter->nfc_lock);
@@ -6491,7 +6490,7 @@ static void igb_get_stats64(struct net_device *netdev,
static int igb_change_mtu(struct net_device *netdev, int new_mtu)
{
struct igb_adapter *adapter = netdev_priv(netdev);
- int max_frame = new_mtu + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN;
+ int max_frame = new_mtu + IGB_ETH_PKT_HDR_PAD;
if (adapter->xdp_prog) {
int i;
--
2.20.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH net v3 3/6] igb: XDP extack message on error
2020-10-19 8:05 ` [Intel-wired-lan] " sven.auhagen
@ 2020-10-19 8:05 ` sven.auhagen
-1 siblings, 0 replies; 28+ messages in thread
From: sven.auhagen @ 2020-10-19 8:05 UTC (permalink / raw)
To: anthony.l.nguyen, maciej.fijalkowski, kuba
Cc: davem, intel-wired-lan, netdev, nhorman, sassmann,
sandeep.penigalapati, brouer
From: Sven Auhagen <sven.auhagen@voleatech.de>
Add an extack error message when the RX buffer size is too small
for the frame size.
Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
---
drivers/net/ethernet/intel/igb/igb_main.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 0a9198037b98..088f9ddb0093 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2824,20 +2824,22 @@ static int igb_setup_tc(struct net_device *dev, enum tc_setup_type type,
}
}
-static int igb_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
+static int igb_xdp_setup(struct net_device *dev, struct netdev_bpf *bpf)
{
int i, frame_size = dev->mtu + IGB_ETH_PKT_HDR_PAD;
struct igb_adapter *adapter = netdev_priv(dev);
+ struct bpf_prog *prog = bpf->prog, *old_prog;
bool running = netif_running(dev);
- struct bpf_prog *old_prog;
bool need_reset;
/* verify igb ring attributes are sufficient for XDP */
for (i = 0; i < adapter->num_rx_queues; i++) {
struct igb_ring *ring = adapter->rx_ring[i];
- if (frame_size > igb_rx_bufsz(ring))
+ if (frame_size > igb_rx_bufsz(ring)) {
+ NL_SET_ERR_MSG_MOD(bpf->extack, "The RX buffer size is too small for the frame size");
return -EINVAL;
+ }
}
old_prog = xchg(&adapter->xdp_prog, prog);
@@ -2869,7 +2871,7 @@ static int igb_xdp(struct net_device *dev, struct netdev_bpf *xdp)
{
switch (xdp->command) {
case XDP_SETUP_PROG:
- return igb_xdp_setup(dev, xdp->prog);
+ return igb_xdp_setup(dev, xdp);
default:
return -EINVAL;
}
@@ -6499,7 +6501,7 @@ static int igb_change_mtu(struct net_device *netdev, int new_mtu)
struct igb_ring *ring = adapter->rx_ring[i];
if (max_frame > igb_rx_bufsz(ring)) {
- netdev_warn(adapter->netdev, "Requested MTU size is not supported with XDP\n");
+ netdev_warn(adapter->netdev, "Requested MTU size is not supported with XDP. Max frame size is %d\n", max_frame);
return -EINVAL;
}
}
--
2.20.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Intel-wired-lan] [PATCH net v3 3/6] igb: XDP extack message on error
@ 2020-10-19 8:05 ` sven.auhagen
0 siblings, 0 replies; 28+ messages in thread
From: sven.auhagen @ 2020-10-19 8:05 UTC (permalink / raw)
To: intel-wired-lan
From: Sven Auhagen <sven.auhagen@voleatech.de>
Add an extack error message when the RX buffer size is too small
for the frame size.
Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
---
drivers/net/ethernet/intel/igb/igb_main.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 0a9198037b98..088f9ddb0093 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2824,20 +2824,22 @@ static int igb_setup_tc(struct net_device *dev, enum tc_setup_type type,
}
}
-static int igb_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
+static int igb_xdp_setup(struct net_device *dev, struct netdev_bpf *bpf)
{
int i, frame_size = dev->mtu + IGB_ETH_PKT_HDR_PAD;
struct igb_adapter *adapter = netdev_priv(dev);
+ struct bpf_prog *prog = bpf->prog, *old_prog;
bool running = netif_running(dev);
- struct bpf_prog *old_prog;
bool need_reset;
/* verify igb ring attributes are sufficient for XDP */
for (i = 0; i < adapter->num_rx_queues; i++) {
struct igb_ring *ring = adapter->rx_ring[i];
- if (frame_size > igb_rx_bufsz(ring))
+ if (frame_size > igb_rx_bufsz(ring)) {
+ NL_SET_ERR_MSG_MOD(bpf->extack, "The RX buffer size is too small for the frame size");
return -EINVAL;
+ }
}
old_prog = xchg(&adapter->xdp_prog, prog);
@@ -2869,7 +2871,7 @@ static int igb_xdp(struct net_device *dev, struct netdev_bpf *xdp)
{
switch (xdp->command) {
case XDP_SETUP_PROG:
- return igb_xdp_setup(dev, xdp->prog);
+ return igb_xdp_setup(dev, xdp);
default:
return -EINVAL;
}
@@ -6499,7 +6501,7 @@ static int igb_change_mtu(struct net_device *netdev, int new_mtu)
struct igb_ring *ring = adapter->rx_ring[i];
if (max_frame > igb_rx_bufsz(ring)) {
- netdev_warn(adapter->netdev, "Requested MTU size is not supported with XDP\n");
+ netdev_warn(adapter->netdev, "Requested MTU size is not supported with XDP. Max frame size is %d\n", max_frame);
return -EINVAL;
}
}
--
2.20.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* RE: [PATCH net v3 3/6] igb: XDP extack message on error
2020-10-19 8:05 ` [Intel-wired-lan] " sven.auhagen
@ 2020-11-11 5:31 ` Penigalapati, Sandeep
-1 siblings, 0 replies; 28+ messages in thread
From: Penigalapati, Sandeep @ 2020-11-11 5:31 UTC (permalink / raw)
To: sven.auhagen, Nguyen, Anthony L, Fijalkowski, Maciej, kuba
Cc: davem, intel-wired-lan, netdev, nhorman, sassmann, brouer
From: sven.auhagen@voleatech.de <sven.auhagen@voleatech.de>
Sent: Monday, October 19, 2020 1:36 PM
To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Fijalkowski, Maciej <maciej.fijalkowski@intel.com>; kuba@kernel.org
Cc: davem@davemloft.net; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; nhorman@redhat.com; sassmann@redhat.com; Penigalapati, Sandeep <sandeep.penigalapati@intel.com>; brouer@redhat.com
Subject: [PATCH net v3 3/6] igb: XDP extack message on error
From: Sven Auhagen <sven.auhagen@voleatech.de>
Add an extack error message when the RX buffer size is too small for the frame size.
Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
---
drivers/net/ethernet/intel/igb/igb_main.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
Tested-by: Sandeep Penigalapati <sandeep.penigalapati@intel.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Intel-wired-lan] [PATCH net v3 3/6] igb: XDP extack message on error
@ 2020-11-11 5:31 ` Penigalapati, Sandeep
0 siblings, 0 replies; 28+ messages in thread
From: Penigalapati, Sandeep @ 2020-11-11 5:31 UTC (permalink / raw)
To: intel-wired-lan
From: sven.auhagen@voleatech.de <sven.auhagen@voleatech.de>
Sent: Monday, October 19, 2020 1:36 PM
To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Fijalkowski, Maciej <maciej.fijalkowski@intel.com>; kuba at kernel.org
Cc: davem at davemloft.net; intel-wired-lan at lists.osuosl.org; netdev at vger.kernel.org; nhorman at redhat.com; sassmann at redhat.com; Penigalapati, Sandeep <sandeep.penigalapati@intel.com>; brouer at redhat.com
Subject: [PATCH net v3 3/6] igb: XDP extack message on error
From: Sven Auhagen <sven.auhagen@voleatech.de>
Add an extack error message when the RX buffer size is too small for the frame size.
Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
---
drivers/net/ethernet/intel/igb/igb_main.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
Tested-by: Sandeep Penigalapati <sandeep.penigalapati@intel.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v3 3/6] igb: XDP extack message on error
2020-10-19 8:05 ` [Intel-wired-lan] " sven.auhagen
@ 2020-11-11 7:11 ` Paul Menzel
-1 siblings, 0 replies; 28+ messages in thread
From: Paul Menzel @ 2020-11-11 7:11 UTC (permalink / raw)
To: sven.auhagen, anthony.l.nguyen, maciej.fijalkowski, kuba
Cc: nhorman, netdev, intel-wired-lan, brouer, davem, sassmann
Dear Sven,
Am 19.10.20 um 10:05 schrieb sven.auhagen@voleatech.de:
> From: Sven Auhagen <sven.auhagen@voleatech.de>
>
> Add an extack error message when the RX buffer size is too small
> for the frame size.
>
> Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
> ---
> drivers/net/ethernet/intel/igb/igb_main.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index 0a9198037b98..088f9ddb0093 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -2824,20 +2824,22 @@ static int igb_setup_tc(struct net_device *dev, enum tc_setup_type type,
> }
> }
>
> -static int igb_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
> +static int igb_xdp_setup(struct net_device *dev, struct netdev_bpf *bpf)
> {
> int i, frame_size = dev->mtu + IGB_ETH_PKT_HDR_PAD;
> struct igb_adapter *adapter = netdev_priv(dev);
> + struct bpf_prog *prog = bpf->prog, *old_prog;
> bool running = netif_running(dev);
> - struct bpf_prog *old_prog;
> bool need_reset;
>
> /* verify igb ring attributes are sufficient for XDP */
> for (i = 0; i < adapter->num_rx_queues; i++) {
> struct igb_ring *ring = adapter->rx_ring[i];
>
> - if (frame_size > igb_rx_bufsz(ring))
> + if (frame_size > igb_rx_bufsz(ring)) {
> + NL_SET_ERR_MSG_MOD(bpf->extack, "The RX buffer size is too small for the frame size");
Could you please also add both size values to the error message?
> return -EINVAL;
> + }
> }
>
> old_prog = xchg(&adapter->xdp_prog, prog);
> @@ -2869,7 +2871,7 @@ static int igb_xdp(struct net_device *dev, struct netdev_bpf *xdp)
> {
> switch (xdp->command) {
> case XDP_SETUP_PROG:
> - return igb_xdp_setup(dev, xdp->prog);
> + return igb_xdp_setup(dev, xdp);
> default:
> return -EINVAL;
> }
> @@ -6499,7 +6501,7 @@ static int igb_change_mtu(struct net_device *netdev, int new_mtu)
> struct igb_ring *ring = adapter->rx_ring[i];
>
> if (max_frame > igb_rx_bufsz(ring)) {
> - netdev_warn(adapter->netdev, "Requested MTU size is not supported with XDP\n");
> + netdev_warn(adapter->netdev, "Requested MTU size is not supported with XDP. Max frame size is %d\n", max_frame);
> return -EINVAL;
> }
> }
>
Kind regards,
Paul
PS: For commit message summaries, statements with verbs in imperative
mood are quite common [1].
> igb: Print XDP extack error on too big frame size
[1]: https://chris.beams.io/posts/git-commit/
^ permalink raw reply [flat|nested] 28+ messages in thread* [Intel-wired-lan] [PATCH net v3 3/6] igb: XDP extack message on error
@ 2020-11-11 7:11 ` Paul Menzel
0 siblings, 0 replies; 28+ messages in thread
From: Paul Menzel @ 2020-11-11 7:11 UTC (permalink / raw)
To: intel-wired-lan
Dear Sven,
Am 19.10.20 um 10:05 schrieb sven.auhagen at voleatech.de:
> From: Sven Auhagen <sven.auhagen@voleatech.de>
>
> Add an extack error message when the RX buffer size is too small
> for the frame size.
>
> Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
> ---
> drivers/net/ethernet/intel/igb/igb_main.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index 0a9198037b98..088f9ddb0093 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -2824,20 +2824,22 @@ static int igb_setup_tc(struct net_device *dev, enum tc_setup_type type,
> }
> }
>
> -static int igb_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
> +static int igb_xdp_setup(struct net_device *dev, struct netdev_bpf *bpf)
> {
> int i, frame_size = dev->mtu + IGB_ETH_PKT_HDR_PAD;
> struct igb_adapter *adapter = netdev_priv(dev);
> + struct bpf_prog *prog = bpf->prog, *old_prog;
> bool running = netif_running(dev);
> - struct bpf_prog *old_prog;
> bool need_reset;
>
> /* verify igb ring attributes are sufficient for XDP */
> for (i = 0; i < adapter->num_rx_queues; i++) {
> struct igb_ring *ring = adapter->rx_ring[i];
>
> - if (frame_size > igb_rx_bufsz(ring))
> + if (frame_size > igb_rx_bufsz(ring)) {
> + NL_SET_ERR_MSG_MOD(bpf->extack, "The RX buffer size is too small for the frame size");
Could you please also add both size values to the error message?
> return -EINVAL;
> + }
> }
>
> old_prog = xchg(&adapter->xdp_prog, prog);
> @@ -2869,7 +2871,7 @@ static int igb_xdp(struct net_device *dev, struct netdev_bpf *xdp)
> {
> switch (xdp->command) {
> case XDP_SETUP_PROG:
> - return igb_xdp_setup(dev, xdp->prog);
> + return igb_xdp_setup(dev, xdp);
> default:
> return -EINVAL;
> }
> @@ -6499,7 +6501,7 @@ static int igb_change_mtu(struct net_device *netdev, int new_mtu)
> struct igb_ring *ring = adapter->rx_ring[i];
>
> if (max_frame > igb_rx_bufsz(ring)) {
> - netdev_warn(adapter->netdev, "Requested MTU size is not supported with XDP\n");
> + netdev_warn(adapter->netdev, "Requested MTU size is not supported with XDP. Max frame size is %d\n", max_frame);
> return -EINVAL;
> }
> }
>
Kind regards,
Paul
PS: For commit message summaries, statements with verbs in imperative
mood are quite common [1].
> igb: Print XDP extack error on too big frame size
[1]: https://chris.beams.io/posts/git-commit/
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [Intel-wired-lan] [PATCH net v3 3/6] igb: XDP extack message on error
2020-11-11 7:11 ` Paul Menzel
@ 2020-11-11 9:39 ` Sven Auhagen
-1 siblings, 0 replies; 28+ messages in thread
From: Sven Auhagen @ 2020-11-11 9:39 UTC (permalink / raw)
To: Paul Menzel
Cc: anthony.l.nguyen, maciej.fijalkowski, kuba, nhorman, netdev,
intel-wired-lan, brouer, davem, sassmann
On Wed, Nov 11, 2020 at 08:11:46AM +0100, Paul Menzel wrote:
> Dear Sven,
>
>
> Am 19.10.20 um 10:05 schrieb sven.auhagen@voleatech.de:
> > From: Sven Auhagen <sven.auhagen@voleatech.de>
> >
> > Add an extack error message when the RX buffer size is too small
> > for the frame size.
> >
> > Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
> > ---
> > drivers/net/ethernet/intel/igb/igb_main.c | 12 +++++++-----
> > 1 file changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> > index 0a9198037b98..088f9ddb0093 100644
> > --- a/drivers/net/ethernet/intel/igb/igb_main.c
> > +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> > @@ -2824,20 +2824,22 @@ static int igb_setup_tc(struct net_device *dev, enum tc_setup_type type,
> > }
> > }
> > -static int igb_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
> > +static int igb_xdp_setup(struct net_device *dev, struct netdev_bpf *bpf)
> > {
> > int i, frame_size = dev->mtu + IGB_ETH_PKT_HDR_PAD;
> > struct igb_adapter *adapter = netdev_priv(dev);
> > + struct bpf_prog *prog = bpf->prog, *old_prog;
> > bool running = netif_running(dev);
> > - struct bpf_prog *old_prog;
> > bool need_reset;
> > /* verify igb ring attributes are sufficient for XDP */
> > for (i = 0; i < adapter->num_rx_queues; i++) {
> > struct igb_ring *ring = adapter->rx_ring[i];
> > - if (frame_size > igb_rx_bufsz(ring))
> > + if (frame_size > igb_rx_bufsz(ring)) {
> > + NL_SET_ERR_MSG_MOD(bpf->extack, "The RX buffer size is too small for the frame size");
>
> Could you please also add both size values to the error message?
Dear Paul,
yes, sure.
I will send a new series with that change.
Best
Sven
>
> > return -EINVAL;
> > + }
> > }
> > old_prog = xchg(&adapter->xdp_prog, prog);
> > @@ -2869,7 +2871,7 @@ static int igb_xdp(struct net_device *dev, struct netdev_bpf *xdp)
> > {
> > switch (xdp->command) {
> > case XDP_SETUP_PROG:
> > - return igb_xdp_setup(dev, xdp->prog);
> > + return igb_xdp_setup(dev, xdp);
> > default:
> > return -EINVAL;
> > }
> > @@ -6499,7 +6501,7 @@ static int igb_change_mtu(struct net_device *netdev, int new_mtu)
> > struct igb_ring *ring = adapter->rx_ring[i];
> > if (max_frame > igb_rx_bufsz(ring)) {
> > - netdev_warn(adapter->netdev, "Requested MTU size is not supported with XDP\n");
> > + netdev_warn(adapter->netdev, "Requested MTU size is not supported with XDP. Max frame size is %d\n", max_frame);
> > return -EINVAL;
> > }
> > }
> >
>
>
> Kind regards,
>
> Paul
>
>
> PS: For commit message summaries, statements with verbs in imperative mood
> are quite common [1].
>
> > igb: Print XDP extack error on too big frame size
>
>
> [1]: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fchris.beams.io%2Fposts%2Fgit-commit%2F&data=04%7C01%7Csven.auhagen%40voleatech.de%7Cc2916e4caf384512cdf808d886110df9%7Cb82a99f679814a7295344d35298f847b%7C0%7C0%7C637406755112287943%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=wBvX6q4trM7FQLp5Nxccqrbo%2ForvF5KG1YG7TRc7cKQ%3D&reserved=0
^ permalink raw reply [flat|nested] 28+ messages in thread* [Intel-wired-lan] [PATCH net v3 3/6] igb: XDP extack message on error
@ 2020-11-11 9:39 ` Sven Auhagen
0 siblings, 0 replies; 28+ messages in thread
From: Sven Auhagen @ 2020-11-11 9:39 UTC (permalink / raw)
To: intel-wired-lan
On Wed, Nov 11, 2020 at 08:11:46AM +0100, Paul Menzel wrote:
> Dear Sven,
>
>
> Am 19.10.20 um 10:05 schrieb sven.auhagen at voleatech.de:
> > From: Sven Auhagen <sven.auhagen@voleatech.de>
> >
> > Add an extack error message when the RX buffer size is too small
> > for the frame size.
> >
> > Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
> > ---
> > drivers/net/ethernet/intel/igb/igb_main.c | 12 +++++++-----
> > 1 file changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> > index 0a9198037b98..088f9ddb0093 100644
> > --- a/drivers/net/ethernet/intel/igb/igb_main.c
> > +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> > @@ -2824,20 +2824,22 @@ static int igb_setup_tc(struct net_device *dev, enum tc_setup_type type,
> > }
> > }
> > -static int igb_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
> > +static int igb_xdp_setup(struct net_device *dev, struct netdev_bpf *bpf)
> > {
> > int i, frame_size = dev->mtu + IGB_ETH_PKT_HDR_PAD;
> > struct igb_adapter *adapter = netdev_priv(dev);
> > + struct bpf_prog *prog = bpf->prog, *old_prog;
> > bool running = netif_running(dev);
> > - struct bpf_prog *old_prog;
> > bool need_reset;
> > /* verify igb ring attributes are sufficient for XDP */
> > for (i = 0; i < adapter->num_rx_queues; i++) {
> > struct igb_ring *ring = adapter->rx_ring[i];
> > - if (frame_size > igb_rx_bufsz(ring))
> > + if (frame_size > igb_rx_bufsz(ring)) {
> > + NL_SET_ERR_MSG_MOD(bpf->extack, "The RX buffer size is too small for the frame size");
>
> Could you please also add both size values to the error message?
Dear Paul,
yes, sure.
I will send a new series with that change.
Best
Sven
>
> > return -EINVAL;
> > + }
> > }
> > old_prog = xchg(&adapter->xdp_prog, prog);
> > @@ -2869,7 +2871,7 @@ static int igb_xdp(struct net_device *dev, struct netdev_bpf *xdp)
> > {
> > switch (xdp->command) {
> > case XDP_SETUP_PROG:
> > - return igb_xdp_setup(dev, xdp->prog);
> > + return igb_xdp_setup(dev, xdp);
> > default:
> > return -EINVAL;
> > }
> > @@ -6499,7 +6501,7 @@ static int igb_change_mtu(struct net_device *netdev, int new_mtu)
> > struct igb_ring *ring = adapter->rx_ring[i];
> > if (max_frame > igb_rx_bufsz(ring)) {
> > - netdev_warn(adapter->netdev, "Requested MTU size is not supported with XDP\n");
> > + netdev_warn(adapter->netdev, "Requested MTU size is not supported with XDP. Max frame size is %d\n", max_frame);
> > return -EINVAL;
> > }
> > }
> >
>
>
> Kind regards,
>
> Paul
>
>
> PS: For commit message summaries, statements with verbs in imperative mood
> are quite common [1].
>
> > igb: Print XDP extack error on too big frame size
>
>
> [1]: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fchris.beams.io%2Fposts%2Fgit-commit%2F&data=04%7C01%7Csven.auhagen%40voleatech.de%7Cc2916e4caf384512cdf808d886110df9%7Cb82a99f679814a7295344d35298f847b%7C0%7C0%7C637406755112287943%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=wBvX6q4trM7FQLp5Nxccqrbo%2ForvF5KG1YG7TRc7cKQ%3D&reserved=0
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [Intel-wired-lan] [PATCH net v3 3/6] igb: XDP extack message on error
2020-11-11 9:39 ` Sven Auhagen
@ 2020-11-11 10:15 ` Jesper Dangaard Brouer
-1 siblings, 0 replies; 28+ messages in thread
From: Jesper Dangaard Brouer @ 2020-11-11 10:15 UTC (permalink / raw)
To: Sven Auhagen
Cc: Paul Menzel, anthony.l.nguyen, maciej.fijalkowski, kuba, nhorman,
netdev, intel-wired-lan, davem, sassmann, brouer
On Wed, 11 Nov 2020 10:39:09 +0100
Sven Auhagen <sven.auhagen@voleatech.de> wrote:
> On Wed, Nov 11, 2020 at 08:11:46AM +0100, Paul Menzel wrote:
> > Dear Sven,
> >
> >
> > Am 19.10.20 um 10:05 schrieb sven.auhagen@voleatech.de:
> > > From: Sven Auhagen <sven.auhagen@voleatech.de>
> > >
> > > Add an extack error message when the RX buffer size is too small
> > > for the frame size.
> > >
> > > Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > > Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > > Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > > Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
> > > ---
> > > drivers/net/ethernet/intel/igb/igb_main.c | 12 +++++++-----
> > > 1 file changed, 7 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> > > index 0a9198037b98..088f9ddb0093 100644
> > > --- a/drivers/net/ethernet/intel/igb/igb_main.c
> > > +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> > > @@ -2824,20 +2824,22 @@ static int igb_setup_tc(struct net_device *dev, enum tc_setup_type type,
> > > }
> > > }
> > > -static int igb_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
> > > +static int igb_xdp_setup(struct net_device *dev, struct netdev_bpf *bpf)
> > > {
> > > int i, frame_size = dev->mtu + IGB_ETH_PKT_HDR_PAD;
> > > struct igb_adapter *adapter = netdev_priv(dev);
> > > + struct bpf_prog *prog = bpf->prog, *old_prog;
> > > bool running = netif_running(dev);
> > > - struct bpf_prog *old_prog;
> > > bool need_reset;
> > > /* verify igb ring attributes are sufficient for XDP */
> > > for (i = 0; i < adapter->num_rx_queues; i++) {
> > > struct igb_ring *ring = adapter->rx_ring[i];
> > > - if (frame_size > igb_rx_bufsz(ring))
> > > + if (frame_size > igb_rx_bufsz(ring)) {
> > > + NL_SET_ERR_MSG_MOD(bpf->extack, "The RX buffer size is too small for the frame size");
> >
> > Could you please also add both size values to the error message?
>
> Dear Paul,
>
> yes, sure.
> I will send a new series with that change.
I don't think it is possible to send this extra variable info via
extack (but the macro might have improved since last I checked).
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 28+ messages in thread* [Intel-wired-lan] [PATCH net v3 3/6] igb: XDP extack message on error
@ 2020-11-11 10:15 ` Jesper Dangaard Brouer
0 siblings, 0 replies; 28+ messages in thread
From: Jesper Dangaard Brouer @ 2020-11-11 10:15 UTC (permalink / raw)
To: intel-wired-lan
On Wed, 11 Nov 2020 10:39:09 +0100
Sven Auhagen <sven.auhagen@voleatech.de> wrote:
> On Wed, Nov 11, 2020 at 08:11:46AM +0100, Paul Menzel wrote:
> > Dear Sven,
> >
> >
> > Am 19.10.20 um 10:05 schrieb sven.auhagen at voleatech.de:
> > > From: Sven Auhagen <sven.auhagen@voleatech.de>
> > >
> > > Add an extack error message when the RX buffer size is too small
> > > for the frame size.
> > >
> > > Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > > Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > > Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > > Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
> > > ---
> > > drivers/net/ethernet/intel/igb/igb_main.c | 12 +++++++-----
> > > 1 file changed, 7 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> > > index 0a9198037b98..088f9ddb0093 100644
> > > --- a/drivers/net/ethernet/intel/igb/igb_main.c
> > > +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> > > @@ -2824,20 +2824,22 @@ static int igb_setup_tc(struct net_device *dev, enum tc_setup_type type,
> > > }
> > > }
> > > -static int igb_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
> > > +static int igb_xdp_setup(struct net_device *dev, struct netdev_bpf *bpf)
> > > {
> > > int i, frame_size = dev->mtu + IGB_ETH_PKT_HDR_PAD;
> > > struct igb_adapter *adapter = netdev_priv(dev);
> > > + struct bpf_prog *prog = bpf->prog, *old_prog;
> > > bool running = netif_running(dev);
> > > - struct bpf_prog *old_prog;
> > > bool need_reset;
> > > /* verify igb ring attributes are sufficient for XDP */
> > > for (i = 0; i < adapter->num_rx_queues; i++) {
> > > struct igb_ring *ring = adapter->rx_ring[i];
> > > - if (frame_size > igb_rx_bufsz(ring))
> > > + if (frame_size > igb_rx_bufsz(ring)) {
> > > + NL_SET_ERR_MSG_MOD(bpf->extack, "The RX buffer size is too small for the frame size");
> >
> > Could you please also add both size values to the error message?
>
> Dear Paul,
>
> yes, sure.
> I will send a new series with that change.
I don't think it is possible to send this extra variable info via
extack (but the macro might have improved since last I checked).
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v3 3/6] igb: XDP extack message on error
2020-11-11 7:11 ` Paul Menzel
@ 2020-11-11 10:10 ` Sven Auhagen
-1 siblings, 0 replies; 28+ messages in thread
From: Sven Auhagen @ 2020-11-11 10:10 UTC (permalink / raw)
To: Paul Menzel
Cc: anthony.l.nguyen, maciej.fijalkowski, kuba, nhorman, netdev,
intel-wired-lan, brouer, davem, sassmann
On Wed, Nov 11, 2020 at 08:11:46AM +0100, Paul Menzel wrote:
> Dear Sven,
>
>
> Am 19.10.20 um 10:05 schrieb sven.auhagen@voleatech.de:
> > From: Sven Auhagen <sven.auhagen@voleatech.de>
> >
> > Add an extack error message when the RX buffer size is too small
> > for the frame size.
> >
> > Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
> > ---
> > drivers/net/ethernet/intel/igb/igb_main.c | 12 +++++++-----
> > 1 file changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> > index 0a9198037b98..088f9ddb0093 100644
> > --- a/drivers/net/ethernet/intel/igb/igb_main.c
> > +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> > @@ -2824,20 +2824,22 @@ static int igb_setup_tc(struct net_device *dev, enum tc_setup_type type,
> > }
> > }
> > -static int igb_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
> > +static int igb_xdp_setup(struct net_device *dev, struct netdev_bpf *bpf)
> > {
> > int i, frame_size = dev->mtu + IGB_ETH_PKT_HDR_PAD;
> > struct igb_adapter *adapter = netdev_priv(dev);
> > + struct bpf_prog *prog = bpf->prog, *old_prog;
> > bool running = netif_running(dev);
> > - struct bpf_prog *old_prog;
> > bool need_reset;
> > /* verify igb ring attributes are sufficient for XDP */
> > for (i = 0; i < adapter->num_rx_queues; i++) {
> > struct igb_ring *ring = adapter->rx_ring[i];
> > - if (frame_size > igb_rx_bufsz(ring))
> > + if (frame_size > igb_rx_bufsz(ring)) {
> > + NL_SET_ERR_MSG_MOD(bpf->extack, "The RX buffer size is too small for the frame size");
Dear Paul,
just to verify, NL_SET_ERR_MSG_MOD does not take any variable arguments
for the text to print.
What seems to be the common practive is to add a second log line
with netdev_warn to print out the sizes.
Is that what you are looking for?
Best
Sven
>
> Could you please also add both size values to the error message?
>
> > return -EINVAL;
> > + }
> > }
> > old_prog = xchg(&adapter->xdp_prog, prog);
> > @@ -2869,7 +2871,7 @@ static int igb_xdp(struct net_device *dev, struct netdev_bpf *xdp)
> > {
> > switch (xdp->command) {
> > case XDP_SETUP_PROG:
> > - return igb_xdp_setup(dev, xdp->prog);
> > + return igb_xdp_setup(dev, xdp);
> > default:
> > return -EINVAL;
> > }
> > @@ -6499,7 +6501,7 @@ static int igb_change_mtu(struct net_device *netdev, int new_mtu)
> > struct igb_ring *ring = adapter->rx_ring[i];
> > if (max_frame > igb_rx_bufsz(ring)) {
> > - netdev_warn(adapter->netdev, "Requested MTU size is not supported with XDP\n");
> > + netdev_warn(adapter->netdev, "Requested MTU size is not supported with XDP. Max frame size is %d\n", max_frame);
> > return -EINVAL;
> > }
> > }
> >
>
>
> Kind regards,
>
> Paul
>
>
> PS: For commit message summaries, statements with verbs in imperative mood
> are quite common [1].
>
> > igb: Print XDP extack error on too big frame size
>
>
> [1]: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fchris.beams.io%2Fposts%2Fgit-commit%2F&data=04%7C01%7Csven.auhagen%40voleatech.de%7Cc2916e4caf384512cdf808d886110df9%7Cb82a99f679814a7295344d35298f847b%7C0%7C0%7C637406755112287943%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=wBvX6q4trM7FQLp5Nxccqrbo%2ForvF5KG1YG7TRc7cKQ%3D&reserved=0
^ permalink raw reply [flat|nested] 28+ messages in thread* [Intel-wired-lan] [PATCH net v3 3/6] igb: XDP extack message on error
@ 2020-11-11 10:10 ` Sven Auhagen
0 siblings, 0 replies; 28+ messages in thread
From: Sven Auhagen @ 2020-11-11 10:10 UTC (permalink / raw)
To: intel-wired-lan
On Wed, Nov 11, 2020 at 08:11:46AM +0100, Paul Menzel wrote:
> Dear Sven,
>
>
> Am 19.10.20 um 10:05 schrieb sven.auhagen at voleatech.de:
> > From: Sven Auhagen <sven.auhagen@voleatech.de>
> >
> > Add an extack error message when the RX buffer size is too small
> > for the frame size.
> >
> > Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
> > ---
> > drivers/net/ethernet/intel/igb/igb_main.c | 12 +++++++-----
> > 1 file changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> > index 0a9198037b98..088f9ddb0093 100644
> > --- a/drivers/net/ethernet/intel/igb/igb_main.c
> > +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> > @@ -2824,20 +2824,22 @@ static int igb_setup_tc(struct net_device *dev, enum tc_setup_type type,
> > }
> > }
> > -static int igb_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
> > +static int igb_xdp_setup(struct net_device *dev, struct netdev_bpf *bpf)
> > {
> > int i, frame_size = dev->mtu + IGB_ETH_PKT_HDR_PAD;
> > struct igb_adapter *adapter = netdev_priv(dev);
> > + struct bpf_prog *prog = bpf->prog, *old_prog;
> > bool running = netif_running(dev);
> > - struct bpf_prog *old_prog;
> > bool need_reset;
> > /* verify igb ring attributes are sufficient for XDP */
> > for (i = 0; i < adapter->num_rx_queues; i++) {
> > struct igb_ring *ring = adapter->rx_ring[i];
> > - if (frame_size > igb_rx_bufsz(ring))
> > + if (frame_size > igb_rx_bufsz(ring)) {
> > + NL_SET_ERR_MSG_MOD(bpf->extack, "The RX buffer size is too small for the frame size");
Dear Paul,
just to verify, NL_SET_ERR_MSG_MOD does not take any variable arguments
for the text to print.
What seems to be the common practive is to add a second log line
with netdev_warn to print out the sizes.
Is that what you are looking for?
Best
Sven
>
> Could you please also add both size values to the error message?
>
> > return -EINVAL;
> > + }
> > }
> > old_prog = xchg(&adapter->xdp_prog, prog);
> > @@ -2869,7 +2871,7 @@ static int igb_xdp(struct net_device *dev, struct netdev_bpf *xdp)
> > {
> > switch (xdp->command) {
> > case XDP_SETUP_PROG:
> > - return igb_xdp_setup(dev, xdp->prog);
> > + return igb_xdp_setup(dev, xdp);
> > default:
> > return -EINVAL;
> > }
> > @@ -6499,7 +6501,7 @@ static int igb_change_mtu(struct net_device *netdev, int new_mtu)
> > struct igb_ring *ring = adapter->rx_ring[i];
> > if (max_frame > igb_rx_bufsz(ring)) {
> > - netdev_warn(adapter->netdev, "Requested MTU size is not supported with XDP\n");
> > + netdev_warn(adapter->netdev, "Requested MTU size is not supported with XDP. Max frame size is %d\n", max_frame);
> > return -EINVAL;
> > }
> > }
> >
>
>
> Kind regards,
>
> Paul
>
>
> PS: For commit message summaries, statements with verbs in imperative mood
> are quite common [1].
>
> > igb: Print XDP extack error on too big frame size
>
>
> [1]: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fchris.beams.io%2Fposts%2Fgit-commit%2F&data=04%7C01%7Csven.auhagen%40voleatech.de%7Cc2916e4caf384512cdf808d886110df9%7Cb82a99f679814a7295344d35298f847b%7C0%7C0%7C637406755112287943%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=wBvX6q4trM7FQLp5Nxccqrbo%2ForvF5KG1YG7TRc7cKQ%3D&reserved=0
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [Intel-wired-lan] [PATCH net v3 3/6] igb: XDP extack message on error
2020-11-11 10:10 ` Sven Auhagen
@ 2020-11-11 10:35 ` Paul Menzel
-1 siblings, 0 replies; 28+ messages in thread
From: Paul Menzel @ 2020-11-11 10:35 UTC (permalink / raw)
To: Sven Auhagen
Cc: anthony.l.nguyen, maciej.fijalkowski, kuba, nhorman, netdev,
intel-wired-lan, brouer, davem, sassmann
Dear Sven,
Am 11.11.20 um 11:10 schrieb Sven Auhagen:
> On Wed, Nov 11, 2020 at 08:11:46AM +0100, Paul Menzel wrote:
>> Am 19.10.20 um 10:05 schrieb sven.auhagen@voleatech.de:
>>> From: Sven Auhagen <sven.auhagen@voleatech.de>
>>>
>>> Add an extack error message when the RX buffer size is too small
>>> for the frame size.
>>>
>>> Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
>>> Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
>>> Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
>>> Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
>>> ---
>>> drivers/net/ethernet/intel/igb/igb_main.c | 12 +++++++-----
>>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
>>> index 0a9198037b98..088f9ddb0093 100644
>>> --- a/drivers/net/ethernet/intel/igb/igb_main.c
>>> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
>>> @@ -2824,20 +2824,22 @@ static int igb_setup_tc(struct net_device *dev, enum tc_setup_type type,
>>> }
>>> }
>>> -static int igb_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
>>> +static int igb_xdp_setup(struct net_device *dev, struct netdev_bpf *bpf)
>>> {
>>> int i, frame_size = dev->mtu + IGB_ETH_PKT_HDR_PAD;
>>> struct igb_adapter *adapter = netdev_priv(dev);
>>> + struct bpf_prog *prog = bpf->prog, *old_prog;
>>> bool running = netif_running(dev);
>>> - struct bpf_prog *old_prog;
>>> bool need_reset;
>>> /* verify igb ring attributes are sufficient for XDP */
>>> for (i = 0; i < adapter->num_rx_queues; i++) {
>>> struct igb_ring *ring = adapter->rx_ring[i];
>>> - if (frame_size > igb_rx_bufsz(ring))
>>> + if (frame_size > igb_rx_bufsz(ring)) {
>>> + NL_SET_ERR_MSG_MOD(bpf->extack, "The RX buffer size is too small for the frame size");
> just to verify, NL_SET_ERR_MSG_MOD does not take any variable arguments
> for the text to print.
Ah, Jesper remarked that too. Can the macro be extended?
> What seems to be the common practice is to add a second log line
> with netdev_warn to print out the sizes.
>
> Is that what you are looking for?
Yes, though it sounds to cumbersome. So, yes, that’d be great for me,
but up to you, if you think it’s useful.
Kind regards,
Paul
>> Could you please also add both size values to the error message?
>>
>>> return -EINVAL;
>>> + }
>>> }
>>> old_prog = xchg(&adapter->xdp_prog, prog);
>>> @@ -2869,7 +2871,7 @@ static int igb_xdp(struct net_device *dev, struct netdev_bpf *xdp)
>>> {
>>> switch (xdp->command) {
>>> case XDP_SETUP_PROG:
>>> - return igb_xdp_setup(dev, xdp->prog);
>>> + return igb_xdp_setup(dev, xdp);
>>> default:
>>> return -EINVAL;
>>> }
>>> @@ -6499,7 +6501,7 @@ static int igb_change_mtu(struct net_device *netdev, int new_mtu)
>>> struct igb_ring *ring = adapter->rx_ring[i];
>>> if (max_frame > igb_rx_bufsz(ring)) {
>>> - netdev_warn(adapter->netdev, "Requested MTU size is not supported with XDP\n");
>>> + netdev_warn(adapter->netdev, "Requested MTU size is not supported with XDP. Max frame size is %d\n", max_frame);
>>> return -EINVAL;
>>> }
>>> }
>>>
>>
>>
>> Kind regards,
>>
>> Paul
>>
>>
>> PS: For commit message summaries, statements with verbs in imperative mood
>> are quite common [1].
>>
>>> igb: Print XDP extack error on too big frame size
>>
>>
>> [1]: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fchris.beams.io%2Fposts%2Fgit-commit%2F&data=04%7C01%7Csven.auhagen%40voleatech.de%7Cc2916e4caf384512cdf808d886110df9%7Cb82a99f679814a7295344d35298f847b%7C0%7C0%7C637406755112287943%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=wBvX6q4trM7FQLp5Nxccqrbo%2ForvF5KG1YG7TRc7cKQ%3D&reserved=0
^ permalink raw reply [flat|nested] 28+ messages in thread* [Intel-wired-lan] [PATCH net v3 3/6] igb: XDP extack message on error
@ 2020-11-11 10:35 ` Paul Menzel
0 siblings, 0 replies; 28+ messages in thread
From: Paul Menzel @ 2020-11-11 10:35 UTC (permalink / raw)
To: intel-wired-lan
Dear Sven,
Am 11.11.20 um 11:10 schrieb Sven Auhagen:
> On Wed, Nov 11, 2020 at 08:11:46AM +0100, Paul Menzel wrote:
>> Am 19.10.20 um 10:05 schrieb sven.auhagen at voleatech.de:
>>> From: Sven Auhagen <sven.auhagen@voleatech.de>
>>>
>>> Add an extack error message when the RX buffer size is too small
>>> for the frame size.
>>>
>>> Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
>>> Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
>>> Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
>>> Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
>>> ---
>>> drivers/net/ethernet/intel/igb/igb_main.c | 12 +++++++-----
>>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
>>> index 0a9198037b98..088f9ddb0093 100644
>>> --- a/drivers/net/ethernet/intel/igb/igb_main.c
>>> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
>>> @@ -2824,20 +2824,22 @@ static int igb_setup_tc(struct net_device *dev, enum tc_setup_type type,
>>> }
>>> }
>>> -static int igb_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
>>> +static int igb_xdp_setup(struct net_device *dev, struct netdev_bpf *bpf)
>>> {
>>> int i, frame_size = dev->mtu + IGB_ETH_PKT_HDR_PAD;
>>> struct igb_adapter *adapter = netdev_priv(dev);
>>> + struct bpf_prog *prog = bpf->prog, *old_prog;
>>> bool running = netif_running(dev);
>>> - struct bpf_prog *old_prog;
>>> bool need_reset;
>>> /* verify igb ring attributes are sufficient for XDP */
>>> for (i = 0; i < adapter->num_rx_queues; i++) {
>>> struct igb_ring *ring = adapter->rx_ring[i];
>>> - if (frame_size > igb_rx_bufsz(ring))
>>> + if (frame_size > igb_rx_bufsz(ring)) {
>>> + NL_SET_ERR_MSG_MOD(bpf->extack, "The RX buffer size is too small for the frame size");
> just to verify, NL_SET_ERR_MSG_MOD does not take any variable arguments
> for the text to print.
Ah, Jesper remarked that too. Can the macro be extended?
> What seems to be the common practice is to add a second log line
> with netdev_warn to print out the sizes.
>
> Is that what you are looking for?
Yes, though it sounds to cumbersome. So, yes, that?d be great for me,
but up to you, if you think it?s useful.
Kind regards,
Paul
>> Could you please also add both size values to the error message?
>>
>>> return -EINVAL;
>>> + }
>>> }
>>> old_prog = xchg(&adapter->xdp_prog, prog);
>>> @@ -2869,7 +2871,7 @@ static int igb_xdp(struct net_device *dev, struct netdev_bpf *xdp)
>>> {
>>> switch (xdp->command) {
>>> case XDP_SETUP_PROG:
>>> - return igb_xdp_setup(dev, xdp->prog);
>>> + return igb_xdp_setup(dev, xdp);
>>> default:
>>> return -EINVAL;
>>> }
>>> @@ -6499,7 +6501,7 @@ static int igb_change_mtu(struct net_device *netdev, int new_mtu)
>>> struct igb_ring *ring = adapter->rx_ring[i];
>>> if (max_frame > igb_rx_bufsz(ring)) {
>>> - netdev_warn(adapter->netdev, "Requested MTU size is not supported with XDP\n");
>>> + netdev_warn(adapter->netdev, "Requested MTU size is not supported with XDP. Max frame size is %d\n", max_frame);
>>> return -EINVAL;
>>> }
>>> }
>>>
>>
>>
>> Kind regards,
>>
>> Paul
>>
>>
>> PS: For commit message summaries, statements with verbs in imperative mood
>> are quite common [1].
>>
>>> igb: Print XDP extack error on too big frame size
>>
>>
>> [1]: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fchris.beams.io%2Fposts%2Fgit-commit%2F&data=04%7C01%7Csven.auhagen%40voleatech.de%7Cc2916e4caf384512cdf808d886110df9%7Cb82a99f679814a7295344d35298f847b%7C0%7C0%7C637406755112287943%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=wBvX6q4trM7FQLp5Nxccqrbo%2ForvF5KG1YG7TRc7cKQ%3D&reserved=0
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [Intel-wired-lan] [PATCH net v3 3/6] igb: XDP extack message on error
2020-11-11 10:35 ` Paul Menzel
@ 2020-11-11 16:16 ` Sven Auhagen
-1 siblings, 0 replies; 28+ messages in thread
From: Sven Auhagen @ 2020-11-11 16:16 UTC (permalink / raw)
To: Paul Menzel
Cc: anthony.l.nguyen, maciej.fijalkowski, kuba, nhorman, netdev,
intel-wired-lan, brouer, davem, sassmann
On Wed, Nov 11, 2020 at 11:35:44AM +0100, Paul Menzel wrote:
> Dear Sven,
>
>
> Am 11.11.20 um 11:10 schrieb Sven Auhagen:
> > On Wed, Nov 11, 2020 at 08:11:46AM +0100, Paul Menzel wrote:
>
> > > Am 19.10.20 um 10:05 schrieb sven.auhagen@voleatech.de:
> > > > From: Sven Auhagen <sven.auhagen@voleatech.de>
> > > >
> > > > Add an extack error message when the RX buffer size is too small
> > > > for the frame size.
> > > >
> > > > Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > > > Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > > > Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > > > Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
> > > > ---
> > > > drivers/net/ethernet/intel/igb/igb_main.c | 12 +++++++-----
> > > > 1 file changed, 7 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> > > > index 0a9198037b98..088f9ddb0093 100644
> > > > --- a/drivers/net/ethernet/intel/igb/igb_main.c
> > > > +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> > > > @@ -2824,20 +2824,22 @@ static int igb_setup_tc(struct net_device *dev, enum tc_setup_type type,
> > > > }
> > > > }
> > > > -static int igb_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
> > > > +static int igb_xdp_setup(struct net_device *dev, struct netdev_bpf *bpf)
> > > > {
> > > > int i, frame_size = dev->mtu + IGB_ETH_PKT_HDR_PAD;
> > > > struct igb_adapter *adapter = netdev_priv(dev);
> > > > + struct bpf_prog *prog = bpf->prog, *old_prog;
> > > > bool running = netif_running(dev);
> > > > - struct bpf_prog *old_prog;
> > > > bool need_reset;
> > > > /* verify igb ring attributes are sufficient for XDP */
> > > > for (i = 0; i < adapter->num_rx_queues; i++) {
> > > > struct igb_ring *ring = adapter->rx_ring[i];
> > > > - if (frame_size > igb_rx_bufsz(ring))
> > > > + if (frame_size > igb_rx_bufsz(ring)) {
> > > > + NL_SET_ERR_MSG_MOD(bpf->extack, "The RX buffer size is too small for the frame size");
>
> > just to verify, NL_SET_ERR_MSG_MOD does not take any variable arguments
> > for the text to print.
>
> Ah, Jesper remarked that too. Can the macro be extended?
It probably can be not as part of this patch series.
>
> > What seems to be the common practice is to add a second log line
> > with netdev_warn to print out the sizes.
> >
> > Is that what you are looking for?
>
> Yes, though it sounds to cumbersome. So, yes, that’d be great for me, but up
> to you, if you think it’s useful.
>
Let me add a netdev warn here so this patch can move forward
and the information is available.
Best
Sven
>
> Kind regards,
>
> Paul
>
>
> > > Could you please also add both size values to the error message?
> > >
> > > > return -EINVAL;
> > > > + }
> > > > }
> > > > old_prog = xchg(&adapter->xdp_prog, prog);
> > > > @@ -2869,7 +2871,7 @@ static int igb_xdp(struct net_device *dev, struct netdev_bpf *xdp)
> > > > {
> > > > switch (xdp->command) {
> > > > case XDP_SETUP_PROG:
> > > > - return igb_xdp_setup(dev, xdp->prog);
> > > > + return igb_xdp_setup(dev, xdp);
> > > > default:
> > > > return -EINVAL;
> > > > }
> > > > @@ -6499,7 +6501,7 @@ static int igb_change_mtu(struct net_device *netdev, int new_mtu)
> > > > struct igb_ring *ring = adapter->rx_ring[i];
> > > > if (max_frame > igb_rx_bufsz(ring)) {
> > > > - netdev_warn(adapter->netdev, "Requested MTU size is not supported with XDP\n");
> > > > + netdev_warn(adapter->netdev, "Requested MTU size is not supported with XDP. Max frame size is %d\n", max_frame);
> > > > return -EINVAL;
> > > > }
> > > > }
> > > >
> > >
> > >
> > > Kind regards,
> > >
> > > Paul
> > >
> > >
> > > PS: For commit message summaries, statements with verbs in imperative mood
> > > are quite common [1].
> > >
> > > > igb: Print XDP extack error on too big frame size
> > >
> > >
> > > [1]: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fchris.beams.io%2Fposts%2Fgit-commit%2F&data=04%7C01%7Csven.auhagen%40voleatech.de%7C832ea50c701b4d89fd5508d8862d8c92%7Cb82a99f679814a7295344d35298f847b%7C0%7C0%7C637406877489969478%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=ITPcipxk%2Bg%2FR0QRssfiF5lm1K3mGJsB5wFkdF3SqiA0%3D&reserved=0
^ permalink raw reply [flat|nested] 28+ messages in thread* [Intel-wired-lan] [PATCH net v3 3/6] igb: XDP extack message on error
@ 2020-11-11 16:16 ` Sven Auhagen
0 siblings, 0 replies; 28+ messages in thread
From: Sven Auhagen @ 2020-11-11 16:16 UTC (permalink / raw)
To: intel-wired-lan
On Wed, Nov 11, 2020 at 11:35:44AM +0100, Paul Menzel wrote:
> Dear Sven,
>
>
> Am 11.11.20 um 11:10 schrieb Sven Auhagen:
> > On Wed, Nov 11, 2020 at 08:11:46AM +0100, Paul Menzel wrote:
>
> > > Am 19.10.20 um 10:05 schrieb sven.auhagen at voleatech.de:
> > > > From: Sven Auhagen <sven.auhagen@voleatech.de>
> > > >
> > > > Add an extack error message when the RX buffer size is too small
> > > > for the frame size.
> > > >
> > > > Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > > > Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > > > Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > > > Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
> > > > ---
> > > > drivers/net/ethernet/intel/igb/igb_main.c | 12 +++++++-----
> > > > 1 file changed, 7 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> > > > index 0a9198037b98..088f9ddb0093 100644
> > > > --- a/drivers/net/ethernet/intel/igb/igb_main.c
> > > > +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> > > > @@ -2824,20 +2824,22 @@ static int igb_setup_tc(struct net_device *dev, enum tc_setup_type type,
> > > > }
> > > > }
> > > > -static int igb_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
> > > > +static int igb_xdp_setup(struct net_device *dev, struct netdev_bpf *bpf)
> > > > {
> > > > int i, frame_size = dev->mtu + IGB_ETH_PKT_HDR_PAD;
> > > > struct igb_adapter *adapter = netdev_priv(dev);
> > > > + struct bpf_prog *prog = bpf->prog, *old_prog;
> > > > bool running = netif_running(dev);
> > > > - struct bpf_prog *old_prog;
> > > > bool need_reset;
> > > > /* verify igb ring attributes are sufficient for XDP */
> > > > for (i = 0; i < adapter->num_rx_queues; i++) {
> > > > struct igb_ring *ring = adapter->rx_ring[i];
> > > > - if (frame_size > igb_rx_bufsz(ring))
> > > > + if (frame_size > igb_rx_bufsz(ring)) {
> > > > + NL_SET_ERR_MSG_MOD(bpf->extack, "The RX buffer size is too small for the frame size");
>
> > just to verify, NL_SET_ERR_MSG_MOD does not take any variable arguments
> > for the text to print.
>
> Ah, Jesper remarked that too. Can the macro be extended?
It probably can be not as part of this patch series.
>
> > What seems to be the common practice is to add a second log line
> > with netdev_warn to print out the sizes.
> >
> > Is that what you are looking for?
>
> Yes, though it sounds to cumbersome. So, yes, that?d be great for me, but up
> to you, if you think it?s useful.
>
Let me add a netdev warn here so this patch can move forward
and the information is available.
Best
Sven
>
> Kind regards,
>
> Paul
>
>
> > > Could you please also add both size values to the error message?
> > >
> > > > return -EINVAL;
> > > > + }
> > > > }
> > > > old_prog = xchg(&adapter->xdp_prog, prog);
> > > > @@ -2869,7 +2871,7 @@ static int igb_xdp(struct net_device *dev, struct netdev_bpf *xdp)
> > > > {
> > > > switch (xdp->command) {
> > > > case XDP_SETUP_PROG:
> > > > - return igb_xdp_setup(dev, xdp->prog);
> > > > + return igb_xdp_setup(dev, xdp);
> > > > default:
> > > > return -EINVAL;
> > > > }
> > > > @@ -6499,7 +6501,7 @@ static int igb_change_mtu(struct net_device *netdev, int new_mtu)
> > > > struct igb_ring *ring = adapter->rx_ring[i];
> > > > if (max_frame > igb_rx_bufsz(ring)) {
> > > > - netdev_warn(adapter->netdev, "Requested MTU size is not supported with XDP\n");
> > > > + netdev_warn(adapter->netdev, "Requested MTU size is not supported with XDP. Max frame size is %d\n", max_frame);
> > > > return -EINVAL;
> > > > }
> > > > }
> > > >
> > >
> > >
> > > Kind regards,
> > >
> > > Paul
> > >
> > >
> > > PS: For commit message summaries, statements with verbs in imperative mood
> > > are quite common [1].
> > >
> > > > igb: Print XDP extack error on too big frame size
> > >
> > >
> > > [1]: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fchris.beams.io%2Fposts%2Fgit-commit%2F&data=04%7C01%7Csven.auhagen%40voleatech.de%7C832ea50c701b4d89fd5508d8862d8c92%7Cb82a99f679814a7295344d35298f847b%7C0%7C0%7C637406877489969478%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=ITPcipxk%2Bg%2FR0QRssfiF5lm1K3mGJsB5wFkdF3SqiA0%3D&reserved=0
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH net v3 4/6] igb: skb add metasize for xdp
2020-10-19 8:05 ` [Intel-wired-lan] " sven.auhagen
@ 2020-10-19 8:05 ` sven.auhagen
-1 siblings, 0 replies; 28+ messages in thread
From: sven.auhagen @ 2020-10-19 8:05 UTC (permalink / raw)
To: anthony.l.nguyen, maciej.fijalkowski, kuba
Cc: davem, intel-wired-lan, netdev, nhorman, sassmann,
sandeep.penigalapati, brouer
From: Sven Auhagen <sven.auhagen@voleatech.de>
add metasize if it is set in xdp
Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
---
drivers/net/ethernet/intel/igb/igb_main.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 088f9ddb0093..36ff8725fdaf 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -8345,6 +8345,7 @@ static struct sk_buff *igb_build_skb(struct igb_ring *rx_ring,
struct xdp_buff *xdp,
union e1000_adv_rx_desc *rx_desc)
{
+ unsigned int metasize = xdp->data - xdp->data_meta;
#if (PAGE_SIZE < 8192)
unsigned int truesize = igb_rx_pg_size(rx_ring) / 2;
#else
@@ -8366,6 +8367,9 @@ static struct sk_buff *igb_build_skb(struct igb_ring *rx_ring,
skb_reserve(skb, xdp->data - xdp->data_hard_start);
__skb_put(skb, xdp->data_end - xdp->data);
+ if (metasize)
+ skb_metadata_set(skb, metasize);
+
/* pull timestamp out of packet data */
if (igb_test_staterr(rx_desc, E1000_RXDADV_STAT_TSIP)) {
igb_ptp_rx_pktstamp(rx_ring->q_vector, skb->data, skb);
--
2.20.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Intel-wired-lan] [PATCH net v3 4/6] igb: skb add metasize for xdp
@ 2020-10-19 8:05 ` sven.auhagen
0 siblings, 0 replies; 28+ messages in thread
From: sven.auhagen @ 2020-10-19 8:05 UTC (permalink / raw)
To: intel-wired-lan
From: Sven Auhagen <sven.auhagen@voleatech.de>
add metasize if it is set in xdp
Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
---
drivers/net/ethernet/intel/igb/igb_main.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 088f9ddb0093..36ff8725fdaf 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -8345,6 +8345,7 @@ static struct sk_buff *igb_build_skb(struct igb_ring *rx_ring,
struct xdp_buff *xdp,
union e1000_adv_rx_desc *rx_desc)
{
+ unsigned int metasize = xdp->data - xdp->data_meta;
#if (PAGE_SIZE < 8192)
unsigned int truesize = igb_rx_pg_size(rx_ring) / 2;
#else
@@ -8366,6 +8367,9 @@ static struct sk_buff *igb_build_skb(struct igb_ring *rx_ring,
skb_reserve(skb, xdp->data - xdp->data_hard_start);
__skb_put(skb, xdp->data_end - xdp->data);
+ if (metasize)
+ skb_metadata_set(skb, metasize);
+
/* pull timestamp out of packet data */
if (igb_test_staterr(rx_desc, E1000_RXDADV_STAT_TSIP)) {
igb_ptp_rx_pktstamp(rx_ring->q_vector, skb->data, skb);
--
2.20.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH net v3 5/6] igb: use xdp_do_flush
2020-10-19 8:05 ` [Intel-wired-lan] " sven.auhagen
@ 2020-10-19 8:05 ` sven.auhagen
-1 siblings, 0 replies; 28+ messages in thread
From: sven.auhagen @ 2020-10-19 8:05 UTC (permalink / raw)
To: anthony.l.nguyen, maciej.fijalkowski, kuba
Cc: davem, intel-wired-lan, netdev, nhorman, sassmann,
sandeep.penigalapati, brouer
From: Sven Auhagen <sven.auhagen@voleatech.de>
Since it is a new XDP implementation change xdp_do_flush_map
to xdp_do_flush.
Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
---
drivers/net/ethernet/intel/igb/igb_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 36ff8725fdaf..55e708f75187 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -8776,7 +8776,7 @@ static int igb_clean_rx_irq(struct igb_q_vector *q_vector, const int budget)
rx_ring->skb = skb;
if (xdp_xmit & IGB_XDP_REDIR)
- xdp_do_flush_map();
+ xdp_do_flush();
if (xdp_xmit & IGB_XDP_TX) {
struct igb_ring *tx_ring = igb_xdp_tx_queue_mapping(adapter);
--
2.20.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Intel-wired-lan] [PATCH net v3 5/6] igb: use xdp_do_flush
@ 2020-10-19 8:05 ` sven.auhagen
0 siblings, 0 replies; 28+ messages in thread
From: sven.auhagen @ 2020-10-19 8:05 UTC (permalink / raw)
To: intel-wired-lan
From: Sven Auhagen <sven.auhagen@voleatech.de>
Since it is a new XDP implementation change xdp_do_flush_map
to xdp_do_flush.
Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
---
drivers/net/ethernet/intel/igb/igb_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 36ff8725fdaf..55e708f75187 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -8776,7 +8776,7 @@ static int igb_clean_rx_irq(struct igb_q_vector *q_vector, const int budget)
rx_ring->skb = skb;
if (xdp_xmit & IGB_XDP_REDIR)
- xdp_do_flush_map();
+ xdp_do_flush();
if (xdp_xmit & IGB_XDP_TX) {
struct igb_ring *tx_ring = igb_xdp_tx_queue_mapping(adapter);
--
2.20.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH net v3 6/6] igb: avoid transmit queue timeout in xdp path
2020-10-19 8:05 ` [Intel-wired-lan] " sven.auhagen
@ 2020-10-19 8:05 ` sven.auhagen
-1 siblings, 0 replies; 28+ messages in thread
From: sven.auhagen @ 2020-10-19 8:05 UTC (permalink / raw)
To: anthony.l.nguyen, maciej.fijalkowski, kuba
Cc: davem, intel-wired-lan, netdev, nhorman, sassmann,
sandeep.penigalapati, brouer
From: Sven Auhagen <sven.auhagen@voleatech.de>
Since we share the transmit queue with the network stack,
it is possible that we run into a transmit queue timeout.
This will reset the queue.
This happens under high load when XDP is using the
transmit queue pretty much exclusively.
netdev_start_xmit() sets the trans_start variable of the
transmit queue to jiffies which is later utilized by dev_watchdog(),
so to avoid timeout, let stack know that XDP xmit happened by
bumping the trans_start within XDP Tx routines to jiffies.
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
---
drivers/net/ethernet/intel/igb/igb_main.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 55e708f75187..4a082c06f48d 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2916,6 +2916,8 @@ static int igb_xdp_xmit_back(struct igb_adapter *adapter, struct xdp_buff *xdp)
nq = txring_txq(tx_ring);
__netif_tx_lock(nq, cpu);
+ /* Avoid transmit queue timeout since we share it with the slow path */
+ nq->trans_start = jiffies;
ret = igb_xmit_xdp_ring(adapter, tx_ring, xdpf);
__netif_tx_unlock(nq);
@@ -2948,6 +2950,9 @@ static int igb_xdp_xmit(struct net_device *dev, int n,
nq = txring_txq(tx_ring);
__netif_tx_lock(nq, cpu);
+ /* Avoid transmit queue timeout since we share it with the slow path */
+ nq->trans_start = jiffies;
+
for (i = 0; i < n; i++) {
struct xdp_frame *xdpf = frames[i];
int err;
--
2.20.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Intel-wired-lan] [PATCH net v3 6/6] igb: avoid transmit queue timeout in xdp path
@ 2020-10-19 8:05 ` sven.auhagen
0 siblings, 0 replies; 28+ messages in thread
From: sven.auhagen @ 2020-10-19 8:05 UTC (permalink / raw)
To: intel-wired-lan
From: Sven Auhagen <sven.auhagen@voleatech.de>
Since we share the transmit queue with the network stack,
it is possible that we run into a transmit queue timeout.
This will reset the queue.
This happens under high load when XDP is using the
transmit queue pretty much exclusively.
netdev_start_xmit() sets the trans_start variable of the
transmit queue to jiffies which is later utilized by dev_watchdog(),
so to avoid timeout, let stack know that XDP xmit happened by
bumping the trans_start within XDP Tx routines to jiffies.
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
---
drivers/net/ethernet/intel/igb/igb_main.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 55e708f75187..4a082c06f48d 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2916,6 +2916,8 @@ static int igb_xdp_xmit_back(struct igb_adapter *adapter, struct xdp_buff *xdp)
nq = txring_txq(tx_ring);
__netif_tx_lock(nq, cpu);
+ /* Avoid transmit queue timeout since we share it with the slow path */
+ nq->trans_start = jiffies;
ret = igb_xmit_xdp_ring(adapter, tx_ring, xdpf);
__netif_tx_unlock(nq);
@@ -2948,6 +2950,9 @@ static int igb_xdp_xmit(struct net_device *dev, int n,
nq = txring_txq(tx_ring);
__netif_tx_lock(nq, cpu);
+ /* Avoid transmit queue timeout since we share it with the slow path */
+ nq->trans_start = jiffies;
+
for (i = 0; i < n; i++) {
struct xdp_frame *xdpf = frames[i];
int err;
--
2.20.1
^ permalink raw reply related [flat|nested] 28+ messages in thread