* [RFC/RFT net-next] net: ethernet: ti: fix netdevice stats for XDP
@ 2020-02-21 17:05 Lorenzo Bianconi
2020-03-16 20:26 ` Grygorii Strashko
0 siblings, 1 reply; 3+ messages in thread
From: Lorenzo Bianconi @ 2020-02-21 17:05 UTC (permalink / raw)
To: netdev; +Cc: ilias.apalodimas, davem, brouer, lorenzo.bianconi,
grygorii.strashko
Align netdevice statistics when the device is running in XDP mode
to other upstream drivers. In particular reports to user-space rx
packets even if they are not forwarded to the networking stack
(XDP_PASS) but they are redirected (XDP_REDIRECT), dropped (XDP_DROP)
or sent back using the same interface (XDP_TX). This patch allows the
system administrator to very the device is receiving data correctly.
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
- this patch is compile-only tested
---
drivers/net/ethernet/ti/cpsw.c | 4 +---
drivers/net/ethernet/ti/cpsw_new.c | 5 ++---
drivers/net/ethernet/ti/cpsw_priv.c | 13 +++++++++++--
drivers/net/ethernet/ti/cpsw_priv.h | 2 +-
4 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 6ae4a72e6f43..fe3fd33f56f7 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -408,12 +408,10 @@ static void cpsw_rx_handler(void *token, int len, int status)
xdp.rxq = &priv->xdp_rxq[ch];
port = priv->emac_port + cpsw->data.dual_emac;
- ret = cpsw_run_xdp(priv, ch, &xdp, page, port);
+ ret = cpsw_run_xdp(priv, ch, &xdp, page, port, &len);
if (ret != CPSW_XDP_PASS)
goto requeue;
- /* XDP prog might have changed packet data and boundaries */
- len = xdp.data_end - xdp.data;
headroom = xdp.data - xdp.data_hard_start;
/* XDP prog can modify vlan tag, so can't use encap header */
diff --git a/drivers/net/ethernet/ti/cpsw_new.c b/drivers/net/ethernet/ti/cpsw_new.c
index 71215db7934b..050496e814c3 100644
--- a/drivers/net/ethernet/ti/cpsw_new.c
+++ b/drivers/net/ethernet/ti/cpsw_new.c
@@ -349,12 +349,11 @@ static void cpsw_rx_handler(void *token, int len, int status)
xdp.data_hard_start = pa;
xdp.rxq = &priv->xdp_rxq[ch];
- ret = cpsw_run_xdp(priv, ch, &xdp, page, priv->emac_port);
+ ret = cpsw_run_xdp(priv, ch, &xdp, page,
+ priv->emac_port, &len);
if (ret != CPSW_XDP_PASS)
goto requeue;
- /* XDP prog might have changed packet data and boundaries */
- len = xdp.data_end - xdp.data;
headroom = xdp.data - xdp.data_hard_start;
/* XDP prog can modify vlan tag, so can't use encap header */
diff --git a/drivers/net/ethernet/ti/cpsw_priv.c b/drivers/net/ethernet/ti/cpsw_priv.c
index 97a058ca60ac..a41da48db40b 100644
--- a/drivers/net/ethernet/ti/cpsw_priv.c
+++ b/drivers/net/ethernet/ti/cpsw_priv.c
@@ -1317,7 +1317,7 @@ int cpsw_xdp_tx_frame(struct cpsw_priv *priv, struct xdp_frame *xdpf,
}
int cpsw_run_xdp(struct cpsw_priv *priv, int ch, struct xdp_buff *xdp,
- struct page *page, int port)
+ struct page *page, int port, int *len)
{
struct cpsw_common *cpsw = priv->cpsw;
struct net_device *ndev = priv->ndev;
@@ -1335,10 +1335,13 @@ int cpsw_run_xdp(struct cpsw_priv *priv, int ch, struct xdp_buff *xdp,
}
act = bpf_prog_run_xdp(prog, xdp);
+ /* XDP prog might have changed packet data and boundaries */
+ *len = xdp.data_end - xdp.data;
+
switch (act) {
case XDP_PASS:
ret = CPSW_XDP_PASS;
- break;
+ goto out;
case XDP_TX:
xdpf = convert_to_xdp_frame(xdp);
if (unlikely(!xdpf))
@@ -1364,8 +1367,14 @@ int cpsw_run_xdp(struct cpsw_priv *priv, int ch, struct xdp_buff *xdp,
trace_xdp_exception(ndev, prog, act);
/* fall through -- handle aborts by dropping packet */
case XDP_DROP:
+ ndev->stats.rx_bytes += *len;
+ ndev->stats.rx_packets++;
goto drop;
}
+
+ ndev->stats.rx_bytes += *len;
+ ndev->stats.rx_packets++;
+
out:
rcu_read_unlock();
return ret;
diff --git a/drivers/net/ethernet/ti/cpsw_priv.h b/drivers/net/ethernet/ti/cpsw_priv.h
index b8d7b924ee3d..54efd773e033 100644
--- a/drivers/net/ethernet/ti/cpsw_priv.h
+++ b/drivers/net/ethernet/ti/cpsw_priv.h
@@ -439,7 +439,7 @@ int cpsw_ndo_bpf(struct net_device *ndev, struct netdev_bpf *bpf);
int cpsw_xdp_tx_frame(struct cpsw_priv *priv, struct xdp_frame *xdpf,
struct page *page, int port);
int cpsw_run_xdp(struct cpsw_priv *priv, int ch, struct xdp_buff *xdp,
- struct page *page, int port);
+ struct page *page, int port, int *len);
irqreturn_t cpsw_tx_interrupt(int irq, void *dev_id);
irqreturn_t cpsw_rx_interrupt(int irq, void *dev_id);
int cpsw_tx_mq_poll(struct napi_struct *napi_tx, int budget);
--
2.24.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [RFC/RFT net-next] net: ethernet: ti: fix netdevice stats for XDP
2020-02-21 17:05 [RFC/RFT net-next] net: ethernet: ti: fix netdevice stats for XDP Lorenzo Bianconi
@ 2020-03-16 20:26 ` Grygorii Strashko
2020-03-16 21:07 ` Lorenzo Bianconi
0 siblings, 1 reply; 3+ messages in thread
From: Grygorii Strashko @ 2020-03-16 20:26 UTC (permalink / raw)
To: Lorenzo Bianconi, netdev
Cc: ilias.apalodimas, davem, brouer, lorenzo.bianconi
On 21/02/2020 19:05, Lorenzo Bianconi wrote:
> Align netdevice statistics when the device is running in XDP mode
> to other upstream drivers. In particular reports to user-space rx
> packets even if they are not forwarded to the networking stack
> (XDP_PASS) but they are redirected (XDP_REDIRECT), dropped (XDP_DROP)
> or sent back using the same interface (XDP_TX). This patch allows the
> system administrator to very the device is receiving data correctly.
I've tested it with xdp-tutorial:
drop: ip link set dev eth0 xdp obj ./packet01-parsing/xdp_prog_kern.o sec xdp_packet_parser
tx: ip link set dev eth0 xdp obj ./packet-solutions/xdp_prog_kern_03.o sec xdp_icmp_echo
And see statistic incremented.
In my opinion, it looks a little bit inconsistent if RX/TX packet/bytes is updated,
but ndev->stats.rx_dropped is not.
>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> - this patch is compile-only tested
> ---
> drivers/net/ethernet/ti/cpsw.c | 4 +---
> drivers/net/ethernet/ti/cpsw_new.c | 5 ++---
> drivers/net/ethernet/ti/cpsw_priv.c | 13 +++++++++++--
> drivers/net/ethernet/ti/cpsw_priv.h | 2 +-
> 4 files changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index 6ae4a72e6f43..fe3fd33f56f7 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
> @@ -408,12 +408,10 @@ static void cpsw_rx_handler(void *token, int len, int status)
> xdp.rxq = &priv->xdp_rxq[ch];
>
> port = priv->emac_port + cpsw->data.dual_emac;
> - ret = cpsw_run_xdp(priv, ch, &xdp, page, port);
> + ret = cpsw_run_xdp(priv, ch, &xdp, page, port, &len);
> if (ret != CPSW_XDP_PASS)
> goto requeue;
>
> - /* XDP prog might have changed packet data and boundaries */
> - len = xdp.data_end - xdp.data;
> headroom = xdp.data - xdp.data_hard_start;
>
> /* XDP prog can modify vlan tag, so can't use encap header */
> diff --git a/drivers/net/ethernet/ti/cpsw_new.c b/drivers/net/ethernet/ti/cpsw_new.c
> index 71215db7934b..050496e814c3 100644
> --- a/drivers/net/ethernet/ti/cpsw_new.c
> +++ b/drivers/net/ethernet/ti/cpsw_new.c
> @@ -349,12 +349,11 @@ static void cpsw_rx_handler(void *token, int len, int status)
> xdp.data_hard_start = pa;
> xdp.rxq = &priv->xdp_rxq[ch];
>
> - ret = cpsw_run_xdp(priv, ch, &xdp, page, priv->emac_port);
> + ret = cpsw_run_xdp(priv, ch, &xdp, page,
> + priv->emac_port, &len);
> if (ret != CPSW_XDP_PASS)
> goto requeue;
>
> - /* XDP prog might have changed packet data and boundaries */
> - len = xdp.data_end - xdp.data;
> headroom = xdp.data - xdp.data_hard_start;
>
> /* XDP prog can modify vlan tag, so can't use encap header */
> diff --git a/drivers/net/ethernet/ti/cpsw_priv.c b/drivers/net/ethernet/ti/cpsw_priv.c
> index 97a058ca60ac..a41da48db40b 100644
> --- a/drivers/net/ethernet/ti/cpsw_priv.c
> +++ b/drivers/net/ethernet/ti/cpsw_priv.c
> @@ -1317,7 +1317,7 @@ int cpsw_xdp_tx_frame(struct cpsw_priv *priv, struct xdp_frame *xdpf,
> }
>
> int cpsw_run_xdp(struct cpsw_priv *priv, int ch, struct xdp_buff *xdp,
> - struct page *page, int port)
> + struct page *page, int port, int *len)
> {
> struct cpsw_common *cpsw = priv->cpsw;
> struct net_device *ndev = priv->ndev;
> @@ -1335,10 +1335,13 @@ int cpsw_run_xdp(struct cpsw_priv *priv, int ch, struct xdp_buff *xdp,
> }
>
> act = bpf_prog_run_xdp(prog, xdp);
> + /* XDP prog might have changed packet data and boundaries */
> + *len = xdp.data_end - xdp.data;
it should be
+ *len = xdp->data_end - xdp->data;
> +
> switch (act) {
> case XDP_PASS:
> ret = CPSW_XDP_PASS;
> - break;
> + goto out;
> case XDP_TX:
> xdpf = convert_to_xdp_frame(xdp);
> if (unlikely(!xdpf))
> @@ -1364,8 +1367,14 @@ int cpsw_run_xdp(struct cpsw_priv *priv, int ch, struct xdp_buff *xdp,
> trace_xdp_exception(ndev, prog, act);
> /* fall through -- handle aborts by dropping packet */
> case XDP_DROP:
> + ndev->stats.rx_bytes += *len;
> + ndev->stats.rx_packets++;
> goto drop;
> }
> +
> + ndev->stats.rx_bytes += *len;
> + ndev->stats.rx_packets++;
> +
> out:
> rcu_read_unlock();
> return ret;
> diff --git a/drivers/net/ethernet/ti/cpsw_priv.h b/drivers/net/ethernet/ti/cpsw_priv.h
> index b8d7b924ee3d..54efd773e033 100644
> --- a/drivers/net/ethernet/ti/cpsw_priv.h
> +++ b/drivers/net/ethernet/ti/cpsw_priv.h
> @@ -439,7 +439,7 @@ int cpsw_ndo_bpf(struct net_device *ndev, struct netdev_bpf *bpf);
> int cpsw_xdp_tx_frame(struct cpsw_priv *priv, struct xdp_frame *xdpf,
> struct page *page, int port);
> int cpsw_run_xdp(struct cpsw_priv *priv, int ch, struct xdp_buff *xdp,
> - struct page *page, int port);
> + struct page *page, int port, int *len);
> irqreturn_t cpsw_tx_interrupt(int irq, void *dev_id);
> irqreturn_t cpsw_rx_interrupt(int irq, void *dev_id);
> int cpsw_tx_mq_poll(struct napi_struct *napi_tx, int budget);
>
--
Best regards,
grygorii
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [RFC/RFT net-next] net: ethernet: ti: fix netdevice stats for XDP
2020-03-16 20:26 ` Grygorii Strashko
@ 2020-03-16 21:07 ` Lorenzo Bianconi
0 siblings, 0 replies; 3+ messages in thread
From: Lorenzo Bianconi @ 2020-03-16 21:07 UTC (permalink / raw)
To: Grygorii Strashko
Cc: Lorenzo Bianconi, netdev, ilias.apalodimas, davem, brouer
[-- Attachment #1: Type: text/plain, Size: 5702 bytes --]
>
>
> On 21/02/2020 19:05, Lorenzo Bianconi wrote:
> > Align netdevice statistics when the device is running in XDP mode
> > to other upstream drivers. In particular reports to user-space rx
> > packets even if they are not forwarded to the networking stack
> > (XDP_PASS) but they are redirected (XDP_REDIRECT), dropped (XDP_DROP)
> > or sent back using the same interface (XDP_TX). This patch allows the
> > system administrator to very the device is receiving data correctly.
>
> I've tested it with xdp-tutorial:
> drop: ip link set dev eth0 xdp obj ./packet01-parsing/xdp_prog_kern.o sec xdp_packet_parser
> tx: ip link set dev eth0 xdp obj ./packet-solutions/xdp_prog_kern_03.o sec xdp_icmp_echo
>
> And see statistic incremented.
>
> In my opinion, it looks a little bit inconsistent if RX/TX packet/bytes is updated,
> but ndev->stats.rx_dropped is not.
Hi Grygorii,
thanks a lot for testing this patch.
The main idea here is to allow the sysadmin to undertand the device is
receiving frames. We will need to add xdp stats to ethtool to give more details
about why we are dropping packets
>
> >
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> > - this patch is compile-only tested
> > ---
> > drivers/net/ethernet/ti/cpsw.c | 4 +---
> > drivers/net/ethernet/ti/cpsw_new.c | 5 ++---
> > drivers/net/ethernet/ti/cpsw_priv.c | 13 +++++++++++--
> > drivers/net/ethernet/ti/cpsw_priv.h | 2 +-
> > 4 files changed, 15 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> > index 6ae4a72e6f43..fe3fd33f56f7 100644
> > --- a/drivers/net/ethernet/ti/cpsw.c
> > +++ b/drivers/net/ethernet/ti/cpsw.c
> > @@ -408,12 +408,10 @@ static void cpsw_rx_handler(void *token, int len, int status)
> > xdp.rxq = &priv->xdp_rxq[ch];
> > port = priv->emac_port + cpsw->data.dual_emac;
> > - ret = cpsw_run_xdp(priv, ch, &xdp, page, port);
> > + ret = cpsw_run_xdp(priv, ch, &xdp, page, port, &len);
> > if (ret != CPSW_XDP_PASS)
> > goto requeue;
> > - /* XDP prog might have changed packet data and boundaries */
> > - len = xdp.data_end - xdp.data;
> > headroom = xdp.data - xdp.data_hard_start;
> > /* XDP prog can modify vlan tag, so can't use encap header */
> > diff --git a/drivers/net/ethernet/ti/cpsw_new.c b/drivers/net/ethernet/ti/cpsw_new.c
> > index 71215db7934b..050496e814c3 100644
> > --- a/drivers/net/ethernet/ti/cpsw_new.c
> > +++ b/drivers/net/ethernet/ti/cpsw_new.c
> > @@ -349,12 +349,11 @@ static void cpsw_rx_handler(void *token, int len, int status)
> > xdp.data_hard_start = pa;
> > xdp.rxq = &priv->xdp_rxq[ch];
> > - ret = cpsw_run_xdp(priv, ch, &xdp, page, priv->emac_port);
> > + ret = cpsw_run_xdp(priv, ch, &xdp, page,
> > + priv->emac_port, &len);
> > if (ret != CPSW_XDP_PASS)
> > goto requeue;
> > - /* XDP prog might have changed packet data and boundaries */
> > - len = xdp.data_end - xdp.data;
> > headroom = xdp.data - xdp.data_hard_start;
> > /* XDP prog can modify vlan tag, so can't use encap header */
> > diff --git a/drivers/net/ethernet/ti/cpsw_priv.c b/drivers/net/ethernet/ti/cpsw_priv.c
> > index 97a058ca60ac..a41da48db40b 100644
> > --- a/drivers/net/ethernet/ti/cpsw_priv.c
> > +++ b/drivers/net/ethernet/ti/cpsw_priv.c
> > @@ -1317,7 +1317,7 @@ int cpsw_xdp_tx_frame(struct cpsw_priv *priv, struct xdp_frame *xdpf,
> > }
> > int cpsw_run_xdp(struct cpsw_priv *priv, int ch, struct xdp_buff *xdp,
> > - struct page *page, int port)
> > + struct page *page, int port, int *len)
> > {
> > struct cpsw_common *cpsw = priv->cpsw;
> > struct net_device *ndev = priv->ndev;
> > @@ -1335,10 +1335,13 @@ int cpsw_run_xdp(struct cpsw_priv *priv, int ch, struct xdp_buff *xdp,
> > }
> > act = bpf_prog_run_xdp(prog, xdp);
> > + /* XDP prog might have changed packet data and boundaries */
> > + *len = xdp.data_end - xdp.data;
>
> it should be
>
> + *len = xdp->data_end - xdp->data;
ops, right sorry
Regards,
Lorenzo
>
>
> > +
> > switch (act) {
> > case XDP_PASS:
> > ret = CPSW_XDP_PASS;
> > - break;
> > + goto out;
> > case XDP_TX:
> > xdpf = convert_to_xdp_frame(xdp);
> > if (unlikely(!xdpf))
> > @@ -1364,8 +1367,14 @@ int cpsw_run_xdp(struct cpsw_priv *priv, int ch, struct xdp_buff *xdp,
> > trace_xdp_exception(ndev, prog, act);
> > /* fall through -- handle aborts by dropping packet */
> > case XDP_DROP:
> > + ndev->stats.rx_bytes += *len;
> > + ndev->stats.rx_packets++;
> > goto drop;
> > }
> > +
> > + ndev->stats.rx_bytes += *len;
> > + ndev->stats.rx_packets++;
> > +
> > out:
> > rcu_read_unlock();
> > return ret;
> > diff --git a/drivers/net/ethernet/ti/cpsw_priv.h b/drivers/net/ethernet/ti/cpsw_priv.h
> > index b8d7b924ee3d..54efd773e033 100644
> > --- a/drivers/net/ethernet/ti/cpsw_priv.h
> > +++ b/drivers/net/ethernet/ti/cpsw_priv.h
> > @@ -439,7 +439,7 @@ int cpsw_ndo_bpf(struct net_device *ndev, struct netdev_bpf *bpf);
> > int cpsw_xdp_tx_frame(struct cpsw_priv *priv, struct xdp_frame *xdpf,
> > struct page *page, int port);
> > int cpsw_run_xdp(struct cpsw_priv *priv, int ch, struct xdp_buff *xdp,
> > - struct page *page, int port);
> > + struct page *page, int port, int *len);
> > irqreturn_t cpsw_tx_interrupt(int irq, void *dev_id);
> > irqreturn_t cpsw_rx_interrupt(int irq, void *dev_id);
> > int cpsw_tx_mq_poll(struct napi_struct *napi_tx, int budget);
> >
>
> --
> Best regards,
> grygorii
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-03-16 21:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-02-21 17:05 [RFC/RFT net-next] net: ethernet: ti: fix netdevice stats for XDP Lorenzo Bianconi
2020-03-16 20:26 ` Grygorii Strashko
2020-03-16 21:07 ` Lorenzo Bianconi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox