netdev - Re: [PATCH net-next 7/8] net/mlx5e: XDP TX forwarding support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160920160054.GA99429@ast-mbp.thefacebook.com>
Date:   Tue, 20 Sep 2016 09:00:55 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Tom Herbert <tom@...bertland.com>
Cc:     Tariq Toukan <ttoukan.linux@...il.com>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Tariq Toukan <tariqt@...lanox.com>,
        "David S. Miller" <davem@...emloft.net>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Eran Ben Elisha <eranbe@...lanox.com>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Rana Shahout <ranas@...lanox.com>
Subject: Re: [PATCH net-next 7/8] net/mlx5e: XDP TX forwarding support

On Tue, Sep 20, 2016 at 08:51:05AM -0700, Tom Herbert wrote:
> On Tue, Sep 20, 2016 at 8:40 AM, Alexei Starovoitov
> <alexei.starovoitov@...il.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.

yes the driver can add another counter, but it's not mandated by xdp side.
Meaning it's up to the driver to have counters and their way of
reporting thems (so far all drivers do it via ethtool).
We don't define any counters via XDP api, since they cannot be defined
in a common way across different nics and hw, and we already have ethtool
which is good enough. Especially in this case 'aborted' counter is
only for debugging. The program should not have 'divide by zero' when
it's written correctly.