On 11/25/2016 01:24 PM, Ferruh Yigit wrote:

On 11/23/2016 7:49 AM, Andrew Rybchenko wrote:

quoted

On 11/23/2016 03:02 AM, Ferruh Yigit wrote:

quoted

On 11/21/2016 3:00 PM, Andrew Rybchenko wrote:

quoted

The patch series adds Solarflare libefx-based network PMD.

This version of the driver supports Solarflare SFN7xxx and SFN8xxx
families of 10/40 Gbps adapters.

libefx is a platform-independent library to implement drivers for
Solarflare network adapters. It provides unified adapter family
independent interface (if possible). FreeBSD [1] and illumos [2]
drivers are built on top of the library.

The patch series could be logically structured into 5 sub-series:
   1. (1) add the driver skeleton including documentation
   2. (2-30) import libefx and include it in build with the latest patch
   3. (31-43) implement minimal device level operations in steps
   4. (44-51) implement Rx subsystem
   5. (52-56) implement Tx subsystem

Functional driver with multi-queue support capable to send and receive
traffic appears with the last patch in the series.

The following design decisions are made during development:

   1. Since libefx uses positive errno return codes, positive errno
      return codes are used inside the driver and coversion to negative
      is done on return from eth_dev_ops callbacks. We think that it
      is the less error-prone way.

   2. Another Solarflare PMD with in-kernel part (for control operations)
      is considered and could be added in the future. Code for data path
      should be shared by these two drivers. libefx-based PMD is put into
      'efx' subdirectory to have a space for another PMD and shared code.

   3. Own event queue (a way to deliver events from HW to host CPU) is
      used for house-keeping (e.g. link status notifications), each Tx
      and each Rx queue. No locks on datapath are requires in this case.

   4. Alarm is used to periodically poll house-keeping event queue.
      The event queue is used to deliver link status change notifications,
      Rx/Tx queue flush events, SRAM events. It is not used on datapath.
      The event queue polling is protected using spin-lock since
      concurrent access from different contexts is possible (e.g. device
      stop when polling alarm is running).

[1] https://svnweb.freebsd.org/base/head/sys/dev/sfxge/common/
[2] https://github.com/illumos/illumos-gate/tree/master/usr/src/uts/common/io/sfxge/common/

---

Andrew Rybchenko (49):
    net/sfc: libefx-based PMD stub sufficient to build and init
    net/sfc: import libefx base
    net/sfc: import libefx register definitions
    net/sfc: import libefx filters support
    net/sfc: import libefx MCDI definition
    net/sfc: import libefx MCDI implementation
    net/sfc: import libefx MCDI logging support
    net/sfc: import libefx MCDI proxy authorization support
    net/sfc: import libefx 5xxx/6xxx family support
    net/sfc: import libefx SFN7xxx family support
    net/sfc: import libefx SFN8xxx family support
    net/sfc: import libefx diagnostics support
    net/sfc: import libefx built-in selftest support
    net/sfc: import libefx software per-queue statistics support
    net/sfc: import libefx PHY flags control support
    net/sfc: import libefx PHY statistics support
    net/sfc: import libefx PHY LEDs control support
    net/sfc: import libefx MAC statistics support
    net/sfc: import libefx event prefetch support
    net/sfc: import libefx Rx scatter support
    net/sfc: import libefx RSS support
    net/sfc: import libefx loopback control support
    net/sfc: import libefx monitors statistics support
    net/sfc: import libefx support to access monitors via MCDI
    net/sfc: import libefx support for Rx packed stream mode
    net/sfc: import libefx NVRAM support
    net/sfc: import libefx VPD support
    net/sfc: import libefx bootrom configuration support
    net/sfc: import libefx licensing support
    net/sfc: implement dummy callback to get device information
    net/sfc: implement driver operation to init device on attach
    net/sfc: add device configure and close stubs
    net/sfc: add device configuration checks
    net/sfc: implement device start and stop operations
    net/sfc: make available resources estimation and allocation
    net/sfc: interrupts support sufficient for event queue init
    net/sfc: implement event queue support
    net/sfc: implement EVQ dummy exception handling
    net/sfc: maintain management event queue
    net/sfc: periodic management EVQ polling using alarm
    net/sfc: minimum port control sufficient to receive traffic
    net/sfc: implement Rx subsystem stubs
    net/sfc: check configured rxmode
    net/sfc: implement Rx queue setup release operations
    net/sfc: calculate Rx buffer size which may be used
    net/sfc: validate Rx queue buffers setup
    net/sfc: implement Rx queue start and stop operations
    net/sfc: implement device callback to Rx burst of packets
    net/sfc: discard scattered packet on Rx correctly

Artem Andreev (2):
    net/sfc: include libefx in build
    net/sfc: implement device operation to retrieve link info

Ivan Malov (5):
    net/sfc: provide basic stubs for Tx subsystem
    net/sfc: add function to check configured Tx mode
    net/sfc: add callbacks to set up and release Tx queues
    net/sfc: implement transmit path start / stop
    net/sfc: add callback to send bursts of packets

Hi Andrew,

Thank you for the patch, I have encounter with a few minor issues, can
you please check them [1]?	

Also folder structure is drivers/net/sfc/efx/<all_src_files>, why /sfc/
layer is created?
sfc is company name (solarflare communications), right? Other driver
folders not structured based on company, what about using
drivers/net/efx/* ?

I've tried to explain it above in item (2):

  >>>

   2. Another Solarflare PMD with in-kernel part (for control operations)
      is considered and could be added in the future. Code for data path
      should be shared by these two drivers. libefx-based PMD is put into
      'efx' subdirectory to have a space for another PMD and shared code.

<<<

So, main reason is to have location for the code shared by two Solarflare
network PMDs. May be it better to relocate when we really have it.
I'm open for other ideas/suggestions.

If there will be another PMD that shares code with current one, the
logic seems good, but I am not sure about start using company names, I
am not against it, just I don't know.

I think that mlx4 and mlx5 are tightly bound to the company name (plus
adapter generation, I guess).

Let's relocate later, this buys some time to think / get feedback on issue.

Do I understand correctly that you suggest to avoid extra level inside 
for now
and relocate later if required? If so, that's fine for me.

As for naming, we think that just "efx" is a bad idea. The prefix is 
occupied by
the libefx functions and driver should use something else. We have chosen
"sfc" mainly to follow naming used in Linux kernel for Solarflare driver
(the first level of Ethernet driver names is company bound in the Linux 
kernel).
If we avoid extra level as discussed above, I think "sfc_efx", "sfcefx" 
(may be it
will look better nearby other drivers) or just "sfc" are fine for us.

quoted

quoted

[1]:
1- There are a few (non-base) checkpatch warnings, can you please check
patch 36, 49, 50 and 55 please?

Thanks, I'll fix spelling in v2.
36, 49 and 55 also ask to check multiple assignments. IMHO, it is the
right usage
of multiple assignment when logically bound variables must have the same
value.

quoted

2- Got following compile issues, not investigated, directly sharing here:

b) for icc getting following warnings:
=======================================
icc: command line warning #10006: ignoring unknown option '-Wno-empty-body'
icc: command line warning #10006: ignoring unknown option
'-Waggregate-return'
icc: command line warning #10006: ignoring unknown option
'-Wbad-function-cast'
icc: command line warning #10006: ignoring unknown option '-Wnested-externs'


c) icc compiler errors:
=======================================
In file included from
.../x86_64-native-linuxapp-icc/include/rte_ethdev.h(185),
                   from .../drivers/net/sfc/efx/sfc.h(35),
                   from .../drivers/net/sfc/efx/sfc.c(37):
.../x86_64-native-linuxapp-icc/include/rte_ether.h(258): warning #2203:
cast discards qualifiers from target type
          uint16_t *from_words = (uint16_t *)(ea_from->addr_bytes);
                                 ^

.../drivers/net/sfc/efx/base/efx_mcdi.c(1157): warning #3179: deprecated
conversion of string literal to char* (should be const char*)
.../drivers/net/sfc/efx/base/ef10_filter.c(1276): warning #188:
enumerated type mixed with another type
                  : "unknown assertion";
                  ^

                  filter_flags = 0;
                               ^

.../drivers/net/sfc/efx/base/efx_mcdi.c(1426): warning #188: enumerated
type mixed with another type
          epp->ep_fixed_port_type =
                                  ^

.../drivers/net/sfc/efx/base/efx_nic.c(556): warning #188: enumerated
type mixed with another type
          enp->en_family = 0;

Yes, I have no ICC compilers. I'll try to fix these warnings, but I
can't be sure without checking it.
Also we cannot claim ICC supported without building and testing the
generated binary.

Please update the code at least to not break the icc compilation,
specially since this PMD will be default enabled. If you prefer I can
verify compilation offline before you send the patchset.

I'll do. I would be thankful if you help with ICC check. Should I send the
patch series directly to you?

Andrew.