netdev - Re: [PATCH net-next 09/10] bnxt_en: Add basic XDP 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: <CACKFLimcOKbewjqsJ9se6_VFD9MfAQ6aATiNnTHNeZOHyp-dmg@mail.gmail.com>
Date:   Mon, 30 Jan 2017 23:33:31 -0800
From:   Michael Chan <michael.chan@...adcom.com>
To:     Jakub Kicinski <kubakici@...pl>
Cc:     David Miller <davem@...emloft.net>, Netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 09/10] bnxt_en: Add basic XDP support.

On Mon, Jan 30, 2017 at 9:04 PM, Jakub Kicinski <kubakici@...pl> wrote:
>
>
>> +     if (netif_running(dev))
>> +             bnxt_close_nic(bp, true, false);
>> +
>> +     old = xchg(&bp->xdp_prog, prog);
>> +     if (old)
>> +             bpf_prog_put(old);
>> +
>> +     if (prog) {
>> +             bnxt_set_rx_skb_mode(bp, true);
>> +     } else {
>> +             bool sh = (bp->flags & BNXT_FLAG_SHARED_RINGS) ? true : false;
>> +             int rx, tx;
>> +
>> +             bnxt_set_rx_skb_mode(bp, false);
>> +             bnxt_get_max_rings(bp, &rx, &tx, sh);
>> +             if (rx > 1) {
>> +                     bp->flags &= ~BNXT_FLAG_NO_AGG_RINGS;
>> +                     bp->dev->hw_features |= NETIF_F_LRO;
>> +             }
>> +     }
>> +     bp->tx_nr_rings_xdp = tx_xdp;
>> +     bp->tx_nr_rings = bp->tx_nr_rings_per_tc * tc + tx_xdp;
>> +     bp->cp_nr_rings = max_t(int, bp->tx_nr_rings, bp->rx_nr_rings);
>> +     bp->num_stat_ctxs = bp->cp_nr_rings;
>> +     bnxt_set_tpa_flags(bp);
>> +     bnxt_set_ring_params(bp);
>> +
>> +     if (netif_running(dev))
>> +             return bnxt_open_nic(bp, true, false);
>
> Mm..  I thought doing open/close like this and risking you won't be
> able to come back up was frowned upon [1].  I must be misunderstanding
> things...

We take these steps:

1. Check to make sure we have enough rings, MSIX, other resources
first.  If we don't have enough resources, ndo_xdp() will return error
code.  Device is still up.
2. We then call firmware to reserve the rings.  If the firmware call
fails, ndo_xdp() will return error code.  Device is still up.
3. Now we close the device and open the device with additional rings.
Yeah, it is possible the open will fail, but it should be quite
unlikely.  In the event that it fails, the netdev will not be in an
inconsistent state.  It will be in the down state.