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  wrote:
> > On Tue, Jan 31, 2017 at 10:36 AM, Andy Gospodarek  
> > 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.

[Alexei Starovoitov]

On Tue, Jan 31, 2017 at 9:33 PM, Andy Gospodarek  wrote:
> On Tue, Jan 31, 2017 at 10:36 AM, Andy Gospodarek  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:36 AM, Andy Gospodarek  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.

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

[Alexei Starovoitov]

On Mon, Jan 30, 2017 at 11:38 PM, Michael Chan
 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.

[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.

[Michael Chan]

On Mon, Jan 30, 2017 at 9:13 PM, Jakub Kicinski  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.

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.

[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.