Re: [PATCH v3 2/4] bus: fsl-mc: add restool userspace support

[Stuart Yoder]

On Wed, Apr 4, 2018 at 7:42 AM, Andrew Lunn  wrote:
>> I hear you.  It is more complicated this way...having all these individual
>> objects vs just a single "bundle" of them that represents a NIC.  But, that's
>> the way the DPAA2 hardware is, and we're implementing kernel support for
>> the hardware as it is.
>
> Hi Stuart
>
> I see we are not making any progress here.
>
> So what i suggest is you post the kernel code and configuration tool
> concept to netdev for a full review. You want reviews from David
> Miller, Jiri Pirko, Jakub Kicinski, David Ahern, etc.

I know Ioana has other feedback she is addressing so a respin will be coming
soon, and she can include those additional reviewers.

Thanks,
Stuart

Re: [PATCH v3 2/4] bus: fsl-mc: add restool userspace support

[Stuart Yoder]

On Tue, Apr 3, 2018 at 8:05 PM, Andrew Lunn  wrote:
>> Suppose you want to create and assign a network interface to a KVM
>> virtual machine, you would do something like the following using
>> a user space tool like restool:
>>-create a new (empty) dprc object
>>-create a new dpni and assign it to the dprc
>>-create a new dpio and assign it to the dprc
>>-create a new dpbp and assign it to the dprc
>>-create a new dpmcp and assign it to the dprc
>>-create a new dpmac and assign it to the dprc
>>-connect the dpni to the dpmac
>
> Hi Stuart
>
> It this connecting to a physical port at the bottom?

Yes.

> If so, i would expect that when you probe the device you just create
> all these for each physical port.

The problem is that there is not just one set of objects to implement a network
interface.  For the highest throughput packet processing you need one dpio
per core.  So, it will depend on what the requirements are.  You might want
multiple dpbp (buffer pools) and set up pools of different size
buffers for different
packet classifications.

You might want to have other objects like a crypto accelerator (dpseci) in the
container as well.

The dprc is a container holding any combination of those objects.  So you have
complete flexibility.

> You then just need to map one of
> them into the KVM, in the same way you map one PCI device into a KVM.
>
> If these are virtual devices, VF devices you would normally do
>
> echo 4 > /sys/class/net//device/sriov_numvfs
>
> on the physical device to create virtual devices.
>
>> The fsl-mc bus and DPAA2 is very NXP-specific, so there doesn't
>> seem to be anything that can be made generic here to provide
>> more common benefit.
>
> Which is why you should try to avoid all of this.  The user knows how
> to use standard linux commands and concepts. They don't want to have
> to learn the inside plumbing of your hardware.

I hear you.  It is more complicated this way...having all these individual
objects vs just a single "bundle" of them that represents a NIC.  But, that's
the way the DPAA2 hardware is, and we're implementing kernel support for
the hardware as it is.

Thanks,
Stuart

Re: [PATCH v3 2/4] bus: fsl-mc: add restool userspace support

[Stuart Yoder]

On Wed, Mar 28, 2018 at 10:43 AM, Arnd Bergmann  wrote:
> On Wed, Mar 28, 2018 at 4:27 PM, Ioana Ciornei  wrote:
>> Hi,
>>
>>>
>>> Hi Ioana,
>>>
>>> So this driver is a direct passthrough to your hardware for passing fixed-
>>> length command/response pairs. Have you considered using a higher-level
>>> interface instead?
>>>
>>> Can you list some of the commands that are passed here as clarification, and
>>> explain what the tradeoffs are that have led to adopting a low-level 
>>> interface
>>> instead of a high-level interface?
>>>
>>> The main downside of the direct passthrough obviously is that you tie your
>>> user space to a particular hardware implementation, while a high-level
>>> abstraction could in principle work across a wider range of hardware 
>>> revisions
>>> or even across multiple vendors implementing the same concept by different
>>> means.
>>
>> If by "higher-level" you mean an implementation where commands are created 
>> by the kernel at userspace's request, then I believe this approach is not 
>> really viable because of the sheer number of possible commands that would 
>> bloat the driver.
>>
>> For example, a DPNI (Data Path Network Interface) can be created using a 
>> command that has the following structure:
>>
>> struct dpni_cmd_create {
>> uint32_t options;
>> uint8_t num_queues;
>> uint8_t num_tcs;
>> uint8_t mac_filter_entries;
>> uint8_t pad1;
>> uint8_t vlan_filter_entries;
>> uint8_t pad2;
>> uint8_t qos_entries;
>> uint8_t pad3;
>> uint16_t fs_entries;
>> };
>>
>> In the above structure, each field has a meaning that the end-user might 
>> want to be able to change according to their particular use-case (not much 
>> is left at its default value).
>> The same level of complexity is encountered for all the commands that 
>> interact with Data Path objects such as DPBP(buffer pools), DPRC(Resource 
>> Container) etc.
>> You can find more examples of commands in restool's repo: 
>> https://github.com/qoriq-open-source/restool/tree/integration/mc_v10
>>
>> In my opinion, an in-kernel implementation that is equivalent in terms of 
>> flexibility will turn
>> into a giant ioctl parser, all while also exposing an userspace API that is 
>> not as simple/easy to use.
>
> (adding the netdev list)
>
> The command you list there seems to be networking related, so instead of
> an ioctl based interface, a high-lever interface would likely use netlink
> for consistency with other drivers. Are all commands for networking
> or are there some that are talking to the device to do something unrelated?
>
> Obviously creating a high-level interface would be a lot of work in the 
> kernel,
> and it only pays off if there are multiple independent users, we wouldn't do
> that for just one driver.
>
> I'm still not convinced either way (high-level or low-level
> interface), but I think
> this needs to be discussed with the networking maintainers. Given the examples
> on the github page you linked to, the high-level user space commands
> based on these ioctls
>
>ls-addni   # adds a network interface
>ls-addmux  # adds a dpdmux
>ls-addsw   # adds an l2switch
>ls-listmac # lists MACs and their connections
>ls-listni  # lists network interfaces and their connections
>
> and I see that you also support the switchdev interface in
> drivers/staging/fsl-dpaa2, which I think does some of the same
> things, presumably by implementing the switchdev API using
> fsl_mc_command low-level interfaces in the kernel.
>
> Is that a correct interpretation? If yes, could we extend switchdev
> or other networking interfaces to fill in whatever those don't handle
> yet?

The wrapper scripts you referenced are not sufficient to show the scope
of what the proposed user space interface is for.  The command list is
not just about networking related objects, as there are quite a few
other types of  objects as well:
dprc - container object representing an fsl-mc bus instance...i.e. other
   objects are attached to this bus
dpio - used for queuing operations towards any accelerator or network
   interface
dpbp - buffer pool object
dpmcp - command portal interface
dpdmai - DMA engine
dpseci - crypto accelerator
dpdcei - compression/decompression accelerator
dpni - network interface
dprtc - real time counter
dpaiop - heterogenous core complex for packet processing offload
dpmac - represents an Ethernet MAC
dpsw - network switch
dpcon - network concentrator
dpci - communication interface

The proposed ioctl interface is about:
   A)  creating and destroying all those object types
   B)  managing the dprc containers they live in, including moving
   objects between containers
   C)  where applicable establishing connections between different
   objects

No _operational_ aspects of these object/devices is being 

[PATCH] net/fsl: simplify Kconfig dependency list for fsl networking

[Stuart Yoder]

make the list of Kconfig dependencies for Freescale
networking more general. Simplify to supported
architectures: ARM, ARM64, PPC, M68K

Signed-off-by: Stuart Yoder stuart.yo...@freescale.com
---
 drivers/net/ethernet/freescale/Kconfig | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/freescale/Kconfig 
b/drivers/net/ethernet/freescale/Kconfig
index ff76d4e..70782d7 100644
--- a/drivers/net/ethernet/freescale/Kconfig
+++ b/drivers/net/ethernet/freescale/Kconfig
@@ -5,9 +5,7 @@
 config NET_VENDOR_FREESCALE
bool Freescale devices
default y
-   depends on FSL_SOC || QUICC_ENGINE || CPM1 || CPM2 || PPC_MPC512x || \
-  M523x || M527x || M5272 || M528x || M520x || M532x || \
-  ARCH_MXC || ARCH_MXS || (PPC_MPC52xx  PPC_BESTCOMM)
+   depends on M68K || PPC || ARM || ARM64
---help---
  If you have a network (Ethernet) card belonging to this class, say Y.
 
-- 
2.3.3

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html