diff mbox

[net] mlx4: fix XDP_TX is acting like XDP_PASS on TX ring full

Message ID 20160916194645.13201.70408.stgit@firesoul
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Jesper Dangaard Brouer Sept. 16, 2016, 7:47 p.m. UTC
The XDP_TX action can fail transmitting the frame in case the TX ring
is full or port is down.  In case of TX failure it should drop the
frame, and not as now call 'break' which is the same as XDP_PASS.

Fixes: 9ecc2d86171a ("net/mlx4_en: add xdp forwarding and data write support")
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>

---
Note, this fix have nothing to do with the page-refcnt bug I just reported.

 drivers/net/ethernet/mellanox/mlx4/en_rx.c |    1 +
 1 file changed, 1 insertion(+)

Comments

Eric Dumazet Sept. 16, 2016, 8 p.m. UTC | #1
On Fri, 2016-09-16 at 21:47 +0200, Jesper Dangaard Brouer wrote:
> The XDP_TX action can fail transmitting the frame in case the TX ring
> is full or port is down.  In case of TX failure it should drop the
> frame, and not as now call 'break' which is the same as XDP_PASS.
> 
> Fixes: 9ecc2d86171a ("net/mlx4_en: add xdp forwarding and data write support")
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> 
> ---
> Note, this fix have nothing to do with the page-refcnt bug I just reported.

Yeah, the e1000 driver proposal patch had the same issue.

> 
>  drivers/net/ethernet/mellanox/mlx4/en_rx.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> index 2040dad8611d..d414c67dfd12 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> @@ -906,6 +906,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
>  							length, tx_index,
>  							&doorbell_pending))
>  					goto consumed;
> +				goto next;
>  				break;

Why keeping this break; then ? ;)

>  			default:
>  				bpf_warn_invalid_xdp_action(act);
>
Jesper Dangaard Brouer Sept. 16, 2016, 8:29 p.m. UTC | #2
On Fri, 16 Sep 2016 13:00:50 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> > diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> > index 2040dad8611d..d414c67dfd12 100644
> > --- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> > +++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> > @@ -906,6 +906,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
> >  							length, tx_index,
> >  							&doorbell_pending))
> >  					goto consumed;
> > +				goto next;
> >  				break;  
> 
> Why keeping this break; then ? ;)

I'll send a V2
diff mbox

Patch

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index 2040dad8611d..d414c67dfd12 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -906,6 +906,7 @@  int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
 							length, tx_index,
 							&doorbell_pending))
 					goto consumed;
+				goto next;
 				break;
 			default:
 				bpf_warn_invalid_xdp_action(act);