[Intel-wired-lan] [PATCH v6 1/2] i40e: add XDP support for pass and drop actions

Björn Töpel bjorn.topel at gmail.com
Tue May 23 20:33:47 UTC 2017


2017-05-23 22:00 GMT+02:00 Duyck, Alexander H <alexander.h.duyck at intel.com>:
> On Tue, 2017-05-23 at 20:51 +0200, Björn Töpel wrote:
>> 2017-05-23 18:51 GMT+02:00 Alexander Duyck <alexander.duyck at gmail.com>:
>> >
>> > On Tue, May 23, 2017 at 12:33 AM, Björn Töpel <bjorn.topel at gmail.com> wrote:
>> > >
>> > > From: Björn Töpel <bjorn.topel at intel.com>
>> > >
>> > > This commit adds basic XDP support for i40e derived NICs. All XDP
>> > > actions will end up in XDP_DROP.
>> > >
>> > > Signed-off-by: Björn Töpel <bjorn.topel at intel.com>
>> >
>> > So I only really see one issue which I pointed out earlier. Basically
>> > the i40e_change_mtu call can't really be dependent on vsi->rx_buf_len
>> > since rx_buf_len is changed as a result of changing the MTU.
>> >
>> > >
>> > > ---
>> > >  drivers/net/ethernet/intel/i40e/i40e.h      |   7 ++
>> > >  drivers/net/ethernet/intel/i40e/i40e_main.c |  75 ++++++++++++++++
>> > >  drivers/net/ethernet/intel/i40e/i40e_txrx.c | 130 +++++++++++++++++++++-------
>> > >  drivers/net/ethernet/intel/i40e/i40e_txrx.h |   1 +
>> > >  4 files changed, 182 insertions(+), 31 deletions(-)
>> > >
>> >
>> > [...]
>> >
>> > >
>> > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> > > index 8d1d3b859af7..c8b1db0ebb9e 100644
>> > > --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
>> > > +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> > > @@ -27,6 +27,7 @@
>> > >  #include <linux/etherdevice.h>
>> > >  #include <linux/of_net.h>
>> > >  #include <linux/pci.h>
>> > > +#include <linux/bpf.h>
>> > >
>> > >  /* Local includes */
>> > >  #include "i40e.h"
>> > > @@ -2408,6 +2409,13 @@ static int i40e_change_mtu(struct net_device *netdev, int new_mtu)
>> > >         struct i40e_vsi *vsi = np->vsi;
>> > >         struct i40e_pf *pf = vsi->back;
>> > >
>> > > +       if (i40e_enabled_xdp_vsi(vsi)) {
>> > > +               int frame_size = new_mtu + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN;
>> > > +
>> > > +               if (frame_size > vsi->rx_buf_len)
>> > > +                       return -EINVAL;
>> > > +       }
>> > > +
>> >
>> > So this code suffers from the same issue that John's ixgbe code did.
>> > You might be better off implementing something like we did with
>> > i40e_vsi_configure_rx. Basically the upper limit can be either 3K or
>> > 2K if the page size greater than 4K or the LEGACY_RX flag is set. You
>> > might look at adding a check here for that instead of just comparing
>> > it to vsi->rx_buf_len since rx_buf_len can change depending on the MTU
>> > size.
>> >
>>
>> You pointed this out in our private conversation, but obviously I
>> didn't get it... :-(
>>
>> So, something in lines of:
>>
>> @@ -2396,6 +2397,18 @@ static void i40e_sync_filters_subtask(struct i40e_pf *pf)
>>  }
>>
>>  /**
>> + * i40e_max_xdp_frame_size - returns the maximum allowed frame size for XDP
>> + * @vsi: the vsi
>> + **/
>> +static int i40e_max_xdp_frame_size(struct i40e_vsi *vsi)
>> +{
>> +       if (PAGE_SIZE >= 8192 || (vsi->back->flags & I40E_FLAG_LEGACY_RX))
>> +               return I40E_RXBUFFER_2048;
>> +       else
>> +               return I40E_RXBUFFER_3072;
>> +}
>> +
>> +/**
>>   * i40e_change_mtu - NDO callback to change the Maximum Transfer Unit
>>   * @netdev: network interface device structure
>>   * @new_mtu: new value for maximum frame size
>> @@ -2408,6 +2421,13 @@ static int i40e_change_mtu(struct net_device
>> *netdev, int new_mtu)
>>         struct i40e_vsi *vsi = np->vsi;
>>         struct i40e_pf *pf = vsi->back;
>>
>> +       if (i40e_enabled_xdp_vsi(vsi)) {
>> +               int frame_size = new_mtu + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN;
>> +
>> +               if (frame_size > i40e_max_xdp_frame_size(vsi))
>> +                       return -EINVAL;
>> +       }
>> +
>>         netdev_info(netdev, "changing MTU from %d to %d\n",
>>                     netdev->mtu, new_mtu);
>>         netdev->mtu = new_mtu;
>>
>>
>>
>> Björn
>
> Yes this is exactly what I had in mind. If you can fold this into your
> patch then I would say we pretty much have this all wrapped up (at
> least until we find something in testing :-)).

Yay! Getting closer... ;-)

I'll spin a v7.


Björn

>
> Thanks.
>
> - Alex


More information about the Intel-wired-lan mailing list