All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jesper Dangaard Brouer <brouer@redhat.com>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Tom Herbert <tom@herbertland.com>,
	Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	Tariq Toukan <ttoukan.linux@gmail.com>,
	Tariq Toukan <tariqt@mellanox.com>,
	"David S. Miller" <davem@davemloft.net>,
	Linux Kernel Network Developers <netdev@vger.kernel.org>,
	Eran Ben Elisha <eranbe@mellanox.com>,
	Saeed Mahameed <saeedm@mellanox.com>,
	Rana Shahout <ranas@mellanox.com>,
	brouer@redhat.com
Subject: Re: [PATCH net-next 7/8] net/mlx5e: XDP TX forwarding support
Date: Tue, 20 Sep 2016 18:06:09 +0200	[thread overview]
Message-ID: <20160920180609.148874b5@redhat.com> (raw)
In-Reply-To: <1474387110.23058.24.camel@edumazet-glaptop3.roam.corp.google.com>

On Tue, 20 Sep 2016 08:58:30 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> On Tue, 2016-09-20 at 08:51 -0700, Tom Herbert wrote:
> > On Tue, Sep 20, 2016 at 8:40 AM, Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:  
> > > On Tue, Sep 20, 2016 at 03:53:10PM +0300, Tariq Toukan wrote:  
> > >> >>>+  case XDP_ABORTED:  
> > >> >>It is not clearly defined, but I believe XDP_ABORTED should also result
> > >> >>in a warning (calling bpf_warn_invalid_xdp_action(act)).  
> > >> I'll add this.  
> > >
> > > Certainly NOT.
> > > XDP_ABORTED is an exception case when program does divide by zero.
> > > It should NOT do bpf_warn. It must drop the packet.
> > > We discussed it several months ago.
> > > See mlx4/en_rx.c for canonical implementation.
> > >  
> > It should at least bump a counter so that the user knows that aborts
> > are happening.  

I agree.

> Same for XDP_TX if/when packet is dropped because output ring is full.

For the XDP_TX case a counter is already incremented[1] but it is a
local ring counter (ring->tx_dropped++).

Do you think we should maintain separate counters for XDP? (to have a
more consistent interface across drivers...)


[1] https://github.com/torvalds/linux/blob/master/drivers/net/ethernet/mellanox/mlx4/en_tx.c#L1181

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

  reply	other threads:[~2016-09-20 16:06 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-19 13:58 [PATCH net-next 0/8] mlx5e XDP support Tariq Toukan
2016-09-19 13:58 ` [PATCH net-next 1/8] net/mlx5e: Build RX SKB on demand Tariq Toukan
2016-09-19 13:58 ` [PATCH net-next 2/8] net/mlx5e: Union RQ RX info per RQ type Tariq Toukan
2016-09-19 13:58 ` [PATCH net-next 3/8] net/mlx5e: Slightly reduce hardware LRO size Tariq Toukan
2016-09-19 13:58 ` [PATCH net-next 4/8] net/mlx5e: Dynamic RQ type infrastructure Tariq Toukan
2016-09-19 13:58 ` [PATCH net-next 5/8] net/mlx5e: XDP fast RX drop bpf programs support Tariq Toukan
2016-09-19 13:58 ` [PATCH net-next 6/8] net/mlx5e: Have a clear separation between different SQ types Tariq Toukan
2016-09-19 13:58 ` [PATCH net-next 7/8] net/mlx5e: XDP TX forwarding support Tariq Toukan
2016-09-20  8:29   ` Jesper Dangaard Brouer
2016-09-20 11:33     ` Jesper Dangaard Brouer
2016-09-20 12:53       ` Tariq Toukan
2016-09-20 15:40         ` Alexei Starovoitov
2016-09-20 15:51           ` Tom Herbert
2016-09-20 15:58             ` Eric Dumazet
2016-09-20 16:06               ` Jesper Dangaard Brouer [this message]
2016-09-20 16:13                 ` Eric Dumazet
2016-09-20 16:27                   ` Jesper Dangaard Brouer
2016-09-20 16:45                     ` Alexei Starovoitov
2016-09-20 16:58                       ` Thomas Graf
2016-09-20 17:39                       ` Eric Dumazet
2016-09-20 18:59                         ` Jesper Dangaard Brouer
2016-09-20 19:21                           ` Tom Herbert
2016-09-20 20:47                           ` Jesper Dangaard Brouer
2016-09-20 21:04                       ` Jesper Dangaard Brouer
2016-09-20 16:00             ` Alexei Starovoitov
2016-09-20 15:57           ` Jesper Dangaard Brouer
2016-09-21  7:57           ` Tariq Toukan
2016-09-21  8:16             ` Jesper Dangaard Brouer
2016-09-19 13:58 ` [PATCH net-next 8/8] net/mlx5e: XDP TX xmit more Tariq Toukan
2016-09-20  7:46   ` Jesper Dangaard Brouer
2016-09-20  8:19     ` Tariq Toukan
2016-09-20  9:26       ` Jesper Dangaard Brouer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160920180609.148874b5@redhat.com \
    --to=brouer@redhat.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=davem@davemloft.net \
    --cc=eranbe@mellanox.com \
    --cc=eric.dumazet@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=ranas@mellanox.com \
    --cc=saeedm@mellanox.com \
    --cc=tariqt@mellanox.com \
    --cc=tom@herbertland.com \
    --cc=ttoukan.linux@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.