Re: [PATCH net-next 00/10] bnxt_en: Add XDP support.

Re: [PATCH net-next 00/10] bnxt_en: Add XDP support.

[Alexei Starovoitov]

On Mon, Jan 30, 2017 at 11:38 PM, Michael Chan
<michael.chan@broadcom.com> wrote:
>
> I need to first figure out what xdp_adjust_head means.  If it is ok,
> I'd like to defer it.

I'd prefer if it's done asap.

mlx4 support added in
commit ea3349a03519 ("mlx4: xdp: Reserve headroom for receiving packet
when XDP prog is active")

mlx5 support added in
commit d8bec2b29a82 ("net/mlx5e: Support bpf_xdp_adjust_head()")

both were relatively small changes to the driver.

Re: [PATCH net-next 00/10] bnxt_en: Add XDP support.

[Alexei Starovoitov]

On Tue, Jan 31, 2017 at 9:33 PM, Andy Gospodarek <andy@greyhouse.net> wrote:
> On Tue, Jan 31, 2017 at 10:36 AM, Andy Gospodarek <andy@greyhouse.net> wrote:
>> On Mon, Jan 30, 2017 at 08:47:47PM -0800, Alexei Starovoitov wrote:
>>> On Mon, Jan 30, 2017 at 08:49:25PM -0500, Michael Chan wrote:
>>> > The first 8 patches refactor the code (rx/tx code paths and ring logic)
>>> > and add the basic infrastructure to support XDP.  The 9th patch adds
>>> > basic ndo_xdp to support XDP_DROP and XDP_PASS only.  The 10th patch
>>> > completes the series with XDP_TX.
>>>
>>> Looks great.
>>> Could you please share performance numbers ?
>>
>> I'll post some later today.
>
> I finally got my system moved around to what I'd hoped would be the
> right spot in my lab, but the system used for generating the traffic
> was only able to send 6Mpps with pktgen, so it was not a great test.
>
> My receiving system with i7-6700 CPU @ 3.40GHz seemed to have no issue
> handling this 6Mpps load -- mpstat showed only one core was ~25%
> utilitzed with all of that servicing softirqs.  The rest of the cores
> were 100% idle.
>
> I'm going to search for other traffic generation tools/systems to make
> sure I can get at least line-rate for the 10GbE cards I was using.

hmm. last time I tried pktgen on bnx2x it was easily doing 14Mpps with burst on.
Have you been using samples/pktgen/pktgen_sample03_burst_single_flow.sh ?
Waiting for this set to land to start benchmarking on bnxt.
So having a baseline will certainly help :)

Thanks!

Re: [PATCH net-next 00/10] bnxt_en: Add XDP support.

[Andy Gospodarek]

On Tue, Jan 31, 2017 at 10:22:12PM -0800, Alexei Starovoitov wrote:
> On Tue, Jan 31, 2017 at 9:33 PM, Andy Gospodarek <andy@greyhouse.net> wrote:
> > On Tue, Jan 31, 2017 at 10:36 AM, Andy Gospodarek <andy@greyhouse.net> wrote:
> >> On Mon, Jan 30, 2017 at 08:47:47PM -0800, Alexei Starovoitov wrote:
> >>> On Mon, Jan 30, 2017 at 08:49:25PM -0500, Michael Chan wrote:
> >>> > The first 8 patches refactor the code (rx/tx code paths and ring logic)
> >>> > and add the basic infrastructure to support XDP.  The 9th patch adds
> >>> > basic ndo_xdp to support XDP_DROP and XDP_PASS only.  The 10th patch
> >>> > completes the series with XDP_TX.
> >>>
> >>> Looks great.
> >>> Could you please share performance numbers ?
> >>
> >> I'll post some later today.
> >
> > I finally got my system moved around to what I'd hoped would be the
> > right spot in my lab, but the system used for generating the traffic
> > was only able to send 6Mpps with pktgen, so it was not a great test.
> >
> > My receiving system with i7-6700 CPU @ 3.40GHz seemed to have no issue
> > handling this 6Mpps load -- mpstat showed only one core was ~25%
> > utilitzed with all of that servicing softirqs.  The rest of the cores
> > were 100% idle.
> >
> > I'm going to search for other traffic generation tools/systems to make
> > sure I can get at least line-rate for the 10GbE cards I was using.
> 
> hmm. last time I tried pktgen on bnx2x it was easily doing 14Mpps with burst on.
> Have you been using samples/pktgen/pktgen_sample03_burst_single_flow.sh ?

Yes I was.  I saw that Brenden has used it to produce some benchmarks, so
that is what I was using as well.

> Waiting for this set to land to start benchmarking on bnxt.
> So having a baseline will certainly help :)

I was hoping to get a better baseline, too.  These results do not seem
to approach the limit of what can be done with XDP on this hardware.
I'm going to try and move some more gear around get to get something
better setup.

Re: [PATCH net-next 00/10] bnxt_en: Add XDP support.

[Andy Gospodarek]

On Mon, Jan 30, 2017 at 08:47:47PM -0800, Alexei Starovoitov wrote:
> On Mon, Jan 30, 2017 at 08:49:25PM -0500, Michael Chan wrote:
> > The first 8 patches refactor the code (rx/tx code paths and ring logic)
> > and add the basic infrastructure to support XDP.  The 9th patch adds
> > basic ndo_xdp to support XDP_DROP and XDP_PASS only.  The 10th patch
> > completes the series with XDP_TX.
> 
> Looks great.
> Could you please share performance numbers ?

I'll post some later today.

> 
> Also please add something like:
>   if (prog && prog->xdp_adjust_head) {
>           netdev_warn(dev, "Does not support bpf_xdp_adjust_head()\n");
>           return -EOPNOTSUPP;
>   }
> unless you plan to add adjut_head support until net-next closes.
> Note, it's must have for load balancer functionality.

Re: [PATCH net-next 00/10] bnxt_en: Add XDP support.

[Andy Gospodarek]

On Tue, Jan 31, 2017 at 10:36 AM, Andy Gospodarek <andy@greyhouse.net> wrote:
> On Mon, Jan 30, 2017 at 08:47:47PM -0800, Alexei Starovoitov wrote:
>> On Mon, Jan 30, 2017 at 08:49:25PM -0500, Michael Chan wrote:
>> > The first 8 patches refactor the code (rx/tx code paths and ring logic)
>> > and add the basic infrastructure to support XDP.  The 9th patch adds
>> > basic ndo_xdp to support XDP_DROP and XDP_PASS only.  The 10th patch
>> > completes the series with XDP_TX.
>>
>> Looks great.
>> Could you please share performance numbers ?
>
> I'll post some later today.

I finally got my system moved around to what I'd hoped would be the
right spot in my lab, but the system used for generating the traffic
was only able to send 6Mpps with pktgen, so it was not a great test.

My receiving system with i7-6700 CPU @ 3.40GHz seemed to have no issue
handling this 6Mpps load -- mpstat showed only one core was ~25%
utilitzed with all of that servicing softirqs.  The rest of the cores
were 100% idle.

I'm going to search for other traffic generation tools/systems to make
sure I can get at least line-rate for the 10GbE cards I was using.

>
>>
>> Also please add something like:
>>   if (prog && prog->xdp_adjust_head) {
>>           netdev_warn(dev, "Does not support bpf_xdp_adjust_head()\n");
>>           return -EOPNOTSUPP;
>>   }
>> unless you plan to add adjut_head support until net-next closes.
>> Note, it's must have for load balancer functionality.

[PATCH net-next 00/10] bnxt_en: Add XDP support.

[Michael Chan]

The first 8 patches refactor the code (rx/tx code paths and ring logic)
and add the basic infrastructure to support XDP.  The 9th patch adds
basic ndo_xdp to support XDP_DROP and XDP_PASS only.  The 10th patch
completes the series with XDP_TX.

Michael Chan (10):
  bnxt_en: Refactor rx SKB function.
  bnxt_en: Add bp->rx_dir field for rx buffer DMA direction.
  bnxt_en: Add RX page mode support.
  bnxt_en: Use event bit map in RX path.
  bnxt_en: Centralize logic to reserve rings.
  bnxt_en: Add tx ring mapping logic.
  bnxt_en: Add a set of TX rings to support XDP.
  bnxt_en: Refactor tx completion path.
  bnxt_en: Add basic XDP support.
  bnxt_en: Add support for XDP_TX action.

 drivers/net/ethernet/broadcom/Kconfig             |   8 +
 drivers/net/ethernet/broadcom/bnxt/Makefile       |   2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt.c         | 347 ++++++++++++++++------
 drivers/net/ethernet/broadcom/bnxt/bnxt.h         |  56 +++-
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c |  53 ++--
 drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c     | 231 ++++++++++++++
 drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.h     |  30 ++
 7 files changed, 599 insertions(+), 128 deletions(-)
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.h

-- 
1.8.3.1

Re: [PATCH net-next 00/10] bnxt_en: Add XDP support.

[Alexei Starovoitov]

On Mon, Jan 30, 2017 at 08:49:25PM -0500, Michael Chan wrote:
> The first 8 patches refactor the code (rx/tx code paths and ring logic)
> and add the basic infrastructure to support XDP.  The 9th patch adds
> basic ndo_xdp to support XDP_DROP and XDP_PASS only.  The 10th patch
> completes the series with XDP_TX.

Looks great.
Could you please share performance numbers ?

Also please add something like:
  if (prog && prog->xdp_adjust_head) {
          netdev_warn(dev, "Does not support bpf_xdp_adjust_head()\n");
          return -EOPNOTSUPP;
  }
unless you plan to add adjut_head support until net-next closes.
Note, it's must have for load balancer functionality.

Re: [PATCH net-next 00/10] bnxt_en: Add XDP support.

[Jakub Kicinski]

On Mon, 30 Jan 2017 20:47:47 -0800, Alexei Starovoitov wrote:
> On Mon, Jan 30, 2017 at 08:49:25PM -0500, Michael Chan wrote:
> > The first 8 patches refactor the code (rx/tx code paths and ring logic)
> > and add the basic infrastructure to support XDP.  The 9th patch adds
> > basic ndo_xdp to support XDP_DROP and XDP_PASS only.  The 10th patch
> > completes the series with XDP_TX.  
> 
> Looks great.
> Could you please share performance numbers ?
> 
> Also please add something like:
>   if (prog && prog->xdp_adjust_head) {
>           netdev_warn(dev, "Does not support bpf_xdp_adjust_head()\n");
>           return -EOPNOTSUPP;
>   }
> unless you plan to add adjut_head support until net-next closes.
> Note, it's must have for load balancer functionality.

I was about to ask whether it's OK at this point to add XDP support
without xdp_adjust_head()?  My understanding was that this is basic
functionality and therefore required?

FWIW the check you requested is there in patch 9.

Re: [PATCH net-next 00/10] bnxt_en: Add XDP support.

[Michael Chan]

On Mon, Jan 30, 2017 at 9:13 PM, Jakub Kicinski <kubakici@wp.pl> wrote:
> On Mon, 30 Jan 2017 20:47:47 -0800, Alexei Starovoitov wrote:
>> On Mon, Jan 30, 2017 at 08:49:25PM -0500, Michael Chan wrote:
>> > The first 8 patches refactor the code (rx/tx code paths and ring logic)
>> > and add the basic infrastructure to support XDP.  The 9th patch adds
>> > basic ndo_xdp to support XDP_DROP and XDP_PASS only.  The 10th patch
>> > completes the series with XDP_TX.
>>
>> Looks great.
>> Could you please share performance numbers ?
>>
>> Also please add something like:
>>   if (prog && prog->xdp_adjust_head) {
>>           netdev_warn(dev, "Does not support bpf_xdp_adjust_head()\n");
>>           return -EOPNOTSUPP;
>>   }
>> unless you plan to add adjut_head support until net-next closes.
>> Note, it's must have for load balancer functionality.
>
> I was about to ask whether it's OK at this point to add XDP support
> without xdp_adjust_head()?  My understanding was that this is basic
> functionality and therefore required?

I need to first figure out what xdp_adjust_head means.  If it is ok,
I'd like to defer it.

>
> FWIW the check you requested is there in patch 9.

Yes, it is in there.  I just copied the same code from other drivers.