mbox series

[v2,0/7] add Intel DCF PMD support

Message ID 20200310065029.40966-1-haiyue.wang@intel.com (mailing list archive)
Headers
Series add Intel DCF PMD support |

Message

Wang, Haiyue March 10, 2020, 6:50 a.m. UTC 
  A DCF (Device Config Function) based approach is proposed where a device
bound to the device's VF0 can act as a sole controlling entity to exercise
advance functionality (such as switch, ACL) for rest of the VFs.

The DCF works as a standalone PMD to support this function, which shares the
ice PMD flow control core function and the iavf virtchnl mailbox core module.

This patchset is based on:
[1] https://patchwork.dpdk.org/cover/66417/ : update ice base code
[2] https://patchwork.dpdk.org/cover/66472/ : iavf share code update

Depends-on: series-8843
Depends-on: series-8855

v2: 
   1. update the iavf patchset link.
   2. split more patches for making this work be more understandable
   3. fix the log function usage, devargs checking from v1.

Haiyue Wang (7):
  net/iavf: stop the PCI probe in DCF mode
  net/ice: add the DCF hardware initialization
  net/ice: initiate to acquire the DCF capability
  net/ice: handle the AdminQ command by DCF
  net/ice: export the DDP definition symbols
  net/ice: handle the PF initialization by DCF
  net/ice: get the VF hardware index in DCF

 doc/guides/nics/ice.rst                |  47 ++
 doc/guides/nics/img/ice_dcf.png        | Bin 0 -> 39168 bytes
 doc/guides/rel_notes/release_20_05.rst |   5 +
 drivers/common/Makefile                |   1 +
 drivers/net/iavf/iavf_ethdev.c         |  43 ++
 drivers/net/ice/Makefile               |   6 +
 drivers/net/ice/ice_dcf.c              | 651 +++++++++++++++++++++++++
 drivers/net/ice/ice_dcf.h              |  61 +++
 drivers/net/ice/ice_dcf_ethdev.c       | 321 ++++++++++++
 drivers/net/ice/ice_dcf_ethdev.h       |  33 ++
 drivers/net/ice/ice_dcf_parent.c       | 344 +++++++++++++
 drivers/net/ice/ice_ethdev.c           |   9 +-
 drivers/net/ice/ice_ethdev.h           |   8 +
 drivers/net/ice/meson.build            |   8 +-
 mk/rte.app.mk                          |   1 +
 15 files changed, 1528 insertions(+), 10 deletions(-)
 create mode 100644 doc/guides/nics/img/ice_dcf.png
 create mode 100644 drivers/net/ice/ice_dcf.c
 create mode 100644 drivers/net/ice/ice_dcf.h
 create mode 100644 drivers/net/ice/ice_dcf_ethdev.c
 create mode 100644 drivers/net/ice/ice_dcf_ethdev.h
 create mode 100644 drivers/net/ice/ice_dcf_parent.c

Comments

Stillwell Jr, Paul M March 13, 2020, 4:19 p.m. UTC | #1 
I'm confused. Shouldn't the DCF be a separate driver since it is a VF, not part of a PF? You are starting to combine PF/VF code and I'm not sure if that is the correct way to go.

Paul

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Haiyue Wang
> Sent: Monday, March 9, 2020 11:50 PM
> To: dev@dpdk.org; Ye, Xiaolong <xiaolong.ye@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>
> Cc: Zhao1, Wei <wei.zhao1@intel.com>; Wang, Haiyue
> <haiyue.wang@intel.com>
> Subject: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> 
> A DCF (Device Config Function) based approach is proposed where a device
> bound to the device's VF0 can act as a sole controlling entity to exercise
> advance functionality (such as switch, ACL) for rest of the VFs.
> 
> The DCF works as a standalone PMD to support this function, which shares
> the ice PMD flow control core function and the iavf virtchnl mailbox core
> module.
> 
> This patchset is based on:
> [1] https://patchwork.dpdk.org/cover/66417/ : update ice base code [2]
> https://patchwork.dpdk.org/cover/66472/ : iavf share code update
> 
> Depends-on: series-8843
> Depends-on: series-8855
> 
> v2:
>    1. update the iavf patchset link.
>    2. split more patches for making this work be more understandable
>    3. fix the log function usage, devargs checking from v1.
> 
> Haiyue Wang (7):
>   net/iavf: stop the PCI probe in DCF mode
>   net/ice: add the DCF hardware initialization
>   net/ice: initiate to acquire the DCF capability
>   net/ice: handle the AdminQ command by DCF
>   net/ice: export the DDP definition symbols
>   net/ice: handle the PF initialization by DCF
>   net/ice: get the VF hardware index in DCF
> 
>  doc/guides/nics/ice.rst                |  47 ++
>  doc/guides/nics/img/ice_dcf.png        | Bin 0 -> 39168 bytes
>  doc/guides/rel_notes/release_20_05.rst |   5 +
>  drivers/common/Makefile                |   1 +
>  drivers/net/iavf/iavf_ethdev.c         |  43 ++
>  drivers/net/ice/Makefile               |   6 +
>  drivers/net/ice/ice_dcf.c              | 651 +++++++++++++++++++++++++
>  drivers/net/ice/ice_dcf.h              |  61 +++
>  drivers/net/ice/ice_dcf_ethdev.c       | 321 ++++++++++++
>  drivers/net/ice/ice_dcf_ethdev.h       |  33 ++
>  drivers/net/ice/ice_dcf_parent.c       | 344 +++++++++++++
>  drivers/net/ice/ice_ethdev.c           |   9 +-
>  drivers/net/ice/ice_ethdev.h           |   8 +
>  drivers/net/ice/meson.build            |   8 +-
>  mk/rte.app.mk                          |   1 +
>  15 files changed, 1528 insertions(+), 10 deletions(-)  create mode 100644
> doc/guides/nics/img/ice_dcf.png  create mode 100644
> drivers/net/ice/ice_dcf.c  create mode 100644 drivers/net/ice/ice_dcf.h
> create mode 100644 drivers/net/ice/ice_dcf_ethdev.c  create mode 100644
> drivers/net/ice/ice_dcf_ethdev.h  create mode 100644
> drivers/net/ice/ice_dcf_parent.c
> 
> --
> 2.25.1
Wang, Haiyue March 13, 2020, 4:25 p.m. UTC | #2 
Hi Paul,

Yes, it's VF (VF hardware initialization like virtchnl). But the flow setting is the
ice PF AdminQ message, so the DCF shares most of ice PMD flow control.

BR,
Haiyue

> -----Original Message-----
> From: Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>
> Sent: Saturday, March 14, 2020 00:19
> To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Ye, Xiaolong <xiaolong.ye@intel.com>; Zhang,
> Qi Z <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Cc: Zhao1, Wei <wei.zhao1@intel.com>; Wang, Haiyue <haiyue.wang@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> 
> I'm confused. Shouldn't the DCF be a separate driver since it is a VF, not part of a PF? You are
> starting to combine PF/VF code and I'm not sure if that is the correct way to go.
> 
> Paul
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Haiyue Wang
> > Sent: Monday, March 9, 2020 11:50 PM
> > To: dev@dpdk.org; Ye, Xiaolong <xiaolong.ye@intel.com>; Zhang, Qi Z
> > <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Xing, Beilei
> > <beilei.xing@intel.com>
> > Cc: Zhao1, Wei <wei.zhao1@intel.com>; Wang, Haiyue
> > <haiyue.wang@intel.com>
> > Subject: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> >
> > A DCF (Device Config Function) based approach is proposed where a device
> > bound to the device's VF0 can act as a sole controlling entity to exercise
> > advance functionality (such as switch, ACL) for rest of the VFs.
> >
> > The DCF works as a standalone PMD to support this function, which shares
> > the ice PMD flow control core function and the iavf virtchnl mailbox core
> > module.
> >
> > This patchset is based on:
> > [1] https://patchwork.dpdk.org/cover/66417/ : update ice base code [2]
> > https://patchwork.dpdk.org/cover/66472/ : iavf share code update
> >
> > Depends-on: series-8843
> > Depends-on: series-8855
> >
> > v2:
> >    1. update the iavf patchset link.
> >    2. split more patches for making this work be more understandable
> >    3. fix the log function usage, devargs checking from v1.
> >
> > Haiyue Wang (7):
> >   net/iavf: stop the PCI probe in DCF mode
> >   net/ice: add the DCF hardware initialization
> >   net/ice: initiate to acquire the DCF capability
> >   net/ice: handle the AdminQ command by DCF
> >   net/ice: export the DDP definition symbols
> >   net/ice: handle the PF initialization by DCF
> >   net/ice: get the VF hardware index in DCF
> >
> >  doc/guides/nics/ice.rst                |  47 ++
> >  doc/guides/nics/img/ice_dcf.png        | Bin 0 -> 39168 bytes
> >  doc/guides/rel_notes/release_20_05.rst |   5 +
> >  drivers/common/Makefile                |   1 +
> >  drivers/net/iavf/iavf_ethdev.c         |  43 ++
> >  drivers/net/ice/Makefile               |   6 +
> >  drivers/net/ice/ice_dcf.c              | 651 +++++++++++++++++++++++++
> >  drivers/net/ice/ice_dcf.h              |  61 +++
> >  drivers/net/ice/ice_dcf_ethdev.c       | 321 ++++++++++++
> >  drivers/net/ice/ice_dcf_ethdev.h       |  33 ++
> >  drivers/net/ice/ice_dcf_parent.c       | 344 +++++++++++++
> >  drivers/net/ice/ice_ethdev.c           |   9 +-
> >  drivers/net/ice/ice_ethdev.h           |   8 +
> >  drivers/net/ice/meson.build            |   8 +-
> >  mk/rte.app.mk                          |   1 +
> >  15 files changed, 1528 insertions(+), 10 deletions(-)  create mode 100644
> > doc/guides/nics/img/ice_dcf.png  create mode 100644
> > drivers/net/ice/ice_dcf.c  create mode 100644 drivers/net/ice/ice_dcf.h
> > create mode 100644 drivers/net/ice/ice_dcf_ethdev.c  create mode 100644
> > drivers/net/ice/ice_dcf_ethdev.h  create mode 100644
> > drivers/net/ice/ice_dcf_parent.c
> >
> > --
> > 2.25.1
>
Stillwell Jr, Paul M March 13, 2020, 4:50 p.m. UTC | #3 
Hi Haiyue,

This statement is confusing to me "But the flow setting is the ice PF AdminQ message, so the DCF shares most of ice PMD flow control." What do you mean by "flow setting"? The way I understand DCF working is that there is a trusted VF (the DCF) that is setting switch rules for other VFs on the same PF. The mechanism for doing that is the DCF sends a virtchnl message to the Linux kernel driver PF to add/delete a switch rule. None of this requires the ice PMD as far as I can tell. This seems like a driver just like the iavf driver; the iavf driver is separate from the ice PMD and it seems like DCF should also be separate.

Paul

> -----Original Message-----
> From: Wang, Haiyue <haiyue.wang@intel.com>
> Sent: Friday, March 13, 2020 9:25 AM
> To: Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>; dev@dpdk.org; Ye,
> Xiaolong <xiaolong.ye@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> Yang, Qiming <qiming.yang@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Cc: Zhao1, Wei <wei.zhao1@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> 
> Hi Paul,
> 
> Yes, it's VF (VF hardware initialization like virtchnl). But the flow setting is the
> ice PF AdminQ message, so the DCF shares most of ice PMD flow control.
> 
> BR,
> Haiyue
> 
> > -----Original Message-----
> > From: Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>
> > Sent: Saturday, March 14, 2020 00:19
> > To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Ye, Xiaolong
> > <xiaolong.ye@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Yang,
> > Qiming <qiming.yang@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> > Cc: Zhao1, Wei <wei.zhao1@intel.com>; Wang, Haiyue
> > <haiyue.wang@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> >
> > I'm confused. Shouldn't the DCF be a separate driver since it is a VF,
> > not part of a PF? You are starting to combine PF/VF code and I'm not sure if
> that is the correct way to go.
> >
> > Paul
> >
> > > -----Original Message-----
> > > From: dev <dev-bounces@dpdk.org> On Behalf Of Haiyue Wang
> > > Sent: Monday, March 9, 2020 11:50 PM
> > > To: dev@dpdk.org; Ye, Xiaolong <xiaolong.ye@intel.com>; Zhang, Qi Z
> > > <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Xing,
> > > Beilei <beilei.xing@intel.com>
> > > Cc: Zhao1, Wei <wei.zhao1@intel.com>; Wang, Haiyue
> > > <haiyue.wang@intel.com>
> > > Subject: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> > >
> > > A DCF (Device Config Function) based approach is proposed where a
> > > device bound to the device's VF0 can act as a sole controlling
> > > entity to exercise advance functionality (such as switch, ACL) for rest of
> the VFs.
> > >
> > > The DCF works as a standalone PMD to support this function, which
> > > shares the ice PMD flow control core function and the iavf virtchnl
> > > mailbox core module.
> > >
> > > This patchset is based on:
> > > [1] https://patchwork.dpdk.org/cover/66417/ : update ice base code
> > > [2] https://patchwork.dpdk.org/cover/66472/ : iavf share code update
> > >
> > > Depends-on: series-8843
> > > Depends-on: series-8855
> > >
> > > v2:
> > >    1. update the iavf patchset link.
> > >    2. split more patches for making this work be more understandable
> > >    3. fix the log function usage, devargs checking from v1.
> > >
> > > Haiyue Wang (7):
> > >   net/iavf: stop the PCI probe in DCF mode
> > >   net/ice: add the DCF hardware initialization
> > >   net/ice: initiate to acquire the DCF capability
> > >   net/ice: handle the AdminQ command by DCF
> > >   net/ice: export the DDP definition symbols
> > >   net/ice: handle the PF initialization by DCF
> > >   net/ice: get the VF hardware index in DCF
> > >
> > >  doc/guides/nics/ice.rst                |  47 ++
> > >  doc/guides/nics/img/ice_dcf.png        | Bin 0 -> 39168 bytes
> > >  doc/guides/rel_notes/release_20_05.rst |   5 +
> > >  drivers/common/Makefile                |   1 +
> > >  drivers/net/iavf/iavf_ethdev.c         |  43 ++
> > >  drivers/net/ice/Makefile               |   6 +
> > >  drivers/net/ice/ice_dcf.c              | 651 +++++++++++++++++++++++++
> > >  drivers/net/ice/ice_dcf.h              |  61 +++
> > >  drivers/net/ice/ice_dcf_ethdev.c       | 321 ++++++++++++
> > >  drivers/net/ice/ice_dcf_ethdev.h       |  33 ++
> > >  drivers/net/ice/ice_dcf_parent.c       | 344 +++++++++++++
> > >  drivers/net/ice/ice_ethdev.c           |   9 +-
> > >  drivers/net/ice/ice_ethdev.h           |   8 +
> > >  drivers/net/ice/meson.build            |   8 +-
> > >  mk/rte.app.mk                          |   1 +
> > >  15 files changed, 1528 insertions(+), 10 deletions(-)  create mode
> > > 100644 doc/guides/nics/img/ice_dcf.png  create mode 100644
> > > drivers/net/ice/ice_dcf.c  create mode 100644
> > > drivers/net/ice/ice_dcf.h create mode 100644
> > > drivers/net/ice/ice_dcf_ethdev.c  create mode 100644
> > > drivers/net/ice/ice_dcf_ethdev.h  create mode 100644
> > > drivers/net/ice/ice_dcf_parent.c
> > >
> > > --
> > > 2.25.1
> >
>
Wang, Haiyue March 13, 2020, 5:05 p.m. UTC | #4 
Hi Paul,

> -----Original Message-----
> From: Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>
> Sent: Saturday, March 14, 2020 00:50
> To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Ye, Xiaolong <xiaolong.ye@intel.com>; Zhang,
> Qi Z <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Cc: Zhao1, Wei <wei.zhao1@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> 
> Hi Haiyue,
> 
> This statement is confusing to me "But the flow setting is the ice PF AdminQ message, so the DCF
> shares most of ice PMD flow control." What do you mean by "flow setting"? 
  It is DPDP rte_flow API calling.

> The way I understand DCF
> working is that there is a trusted VF (the DCF) that is setting switch rules for other VFs on the same
> PF. The mechanism for doing that is the DCF sends a virtchnl message to the Linux kernel driver PF to

DCF needs: 1). virtchnl message is handles by 'dpdk/drivers/common/iavf/', which is the original iavf base code.

> add/delete a switch rule. None of this requires the ice PMD as far as I can tell. This seems like a

DCF needs: 2). add/delete a switch rule.
               rte_flow API --> 'dpdk/drivers/net/ice/ice_switch_filter.c/ice_generic_flow.c' (ice flow framework)
		                     |
                                 `--> 'dpdk/drivers/net/ice/base' ...

So, put it under the ice PMD is the best way.

> driver just like the iavf driver; the iavf driver is separate from the ice PMD and it seems like DCF
> should also be separate.
> 
> Paul
> 
> > -----Original Message-----
> > From: Wang, Haiyue <haiyue.wang@intel.com>
> > Sent: Friday, March 13, 2020 9:25 AM
> > To: Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>; dev@dpdk.org; Ye,
> > Xiaolong <xiaolong.ye@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> > Yang, Qiming <qiming.yang@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> > Cc: Zhao1, Wei <wei.zhao1@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> >
> > Hi Paul,
> >
> > Yes, it's VF (VF hardware initialization like virtchnl). But the flow setting is the
> > ice PF AdminQ message, so the DCF shares most of ice PMD flow control.
> >
> > BR,
> > Haiyue
> >
> > > -----Original Message-----
> > > From: Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>
> > > Sent: Saturday, March 14, 2020 00:19
> > > To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Ye, Xiaolong
> > > <xiaolong.ye@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Yang,
> > > Qiming <qiming.yang@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> > > Cc: Zhao1, Wei <wei.zhao1@intel.com>; Wang, Haiyue
> > > <haiyue.wang@intel.com>
> > > Subject: RE: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> > >
> > > I'm confused. Shouldn't the DCF be a separate driver since it is a VF,
> > > not part of a PF? You are starting to combine PF/VF code and I'm not sure if
> > that is the correct way to go.
> > >
> > > Paul
> > >
> > > > -----Original Message-----
> > > > From: dev <dev-bounces@dpdk.org> On Behalf Of Haiyue Wang
> > > > Sent: Monday, March 9, 2020 11:50 PM
> > > > To: dev@dpdk.org; Ye, Xiaolong <xiaolong.ye@intel.com>; Zhang, Qi Z
> > > > <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Xing,
> > > > Beilei <beilei.xing@intel.com>
> > > > Cc: Zhao1, Wei <wei.zhao1@intel.com>; Wang, Haiyue
> > > > <haiyue.wang@intel.com>
> > > > Subject: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> > > >
> > > > A DCF (Device Config Function) based approach is proposed where a
> > > > device bound to the device's VF0 can act as a sole controlling
> > > > entity to exercise advance functionality (such as switch, ACL) for rest of
> > the VFs.
> > > >
> > > > The DCF works as a standalone PMD to support this function, which
> > > > shares the ice PMD flow control core function and the iavf virtchnl
> > > > mailbox core module.
> > > >
> > > > This patchset is based on:
> > > > [1] https://patchwork.dpdk.org/cover/66417/ : update ice base code
> > > > [2] https://patchwork.dpdk.org/cover/66472/ : iavf share code update
> > > >
> > > > Depends-on: series-8843
> > > > Depends-on: series-8855
> > > >
> > > > v2:
> > > >    1. update the iavf patchset link.
> > > >    2. split more patches for making this work be more understandable
> > > >    3. fix the log function usage, devargs checking from v1.
> > > >
> > > > Haiyue Wang (7):
> > > >   net/iavf: stop the PCI probe in DCF mode
> > > >   net/ice: add the DCF hardware initialization
> > > >   net/ice: initiate to acquire the DCF capability
> > > >   net/ice: handle the AdminQ command by DCF
> > > >   net/ice: export the DDP definition symbols
> > > >   net/ice: handle the PF initialization by DCF
> > > >   net/ice: get the VF hardware index in DCF
> > > >
> > > >  doc/guides/nics/ice.rst                |  47 ++
> > > >  doc/guides/nics/img/ice_dcf.png        | Bin 0 -> 39168 bytes
> > > >  doc/guides/rel_notes/release_20_05.rst |   5 +
> > > >  drivers/common/Makefile                |   1 +
> > > >  drivers/net/iavf/iavf_ethdev.c         |  43 ++
> > > >  drivers/net/ice/Makefile               |   6 +
> > > >  drivers/net/ice/ice_dcf.c              | 651 +++++++++++++++++++++++++
> > > >  drivers/net/ice/ice_dcf.h              |  61 +++
> > > >  drivers/net/ice/ice_dcf_ethdev.c       | 321 ++++++++++++
> > > >  drivers/net/ice/ice_dcf_ethdev.h       |  33 ++
> > > >  drivers/net/ice/ice_dcf_parent.c       | 344 +++++++++++++
> > > >  drivers/net/ice/ice_ethdev.c           |   9 +-
> > > >  drivers/net/ice/ice_ethdev.h           |   8 +
> > > >  drivers/net/ice/meson.build            |   8 +-
> > > >  mk/rte.app.mk                          |   1 +
> > > >  15 files changed, 1528 insertions(+), 10 deletions(-)  create mode
> > > > 100644 doc/guides/nics/img/ice_dcf.png  create mode 100644
> > > > drivers/net/ice/ice_dcf.c  create mode 100644
> > > > drivers/net/ice/ice_dcf.h create mode 100644
> > > > drivers/net/ice/ice_dcf_ethdev.c  create mode 100644
> > > > drivers/net/ice/ice_dcf_ethdev.h  create mode 100644
> > > > drivers/net/ice/ice_dcf_parent.c
> > > >
> > > > --
> > > > 2.25.1
> > >
> >
>
Stillwell Jr, Paul M March 13, 2020, 5:47 p.m. UTC | #5 
OK, I looked at the code further and it seems like what you are doing is you are using the iavf PCI device ID instead of the PF device ID when the user says cap=dcf. This doesn't seem like the right thing to do. Why not modify the iavf code to support being the DCF? Or create a new PMD? You are calling iavf functions from within the ice PMD which seems wrong to me.

Paul

> -----Original Message-----
> From: Wang, Haiyue <haiyue.wang@intel.com>
> Sent: Friday, March 13, 2020 10:05 AM
> To: Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>; dev@dpdk.org; Ye,
> Xiaolong <xiaolong.ye@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> Yang, Qiming <qiming.yang@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Cc: Zhao1, Wei <wei.zhao1@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> 
> Hi Paul,
> 
> > -----Original Message-----
> > From: Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>
> > Sent: Saturday, March 14, 2020 00:50
> > To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Ye, Xiaolong
> > <xiaolong.ye@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Yang,
> > Qiming <qiming.yang@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> > Cc: Zhao1, Wei <wei.zhao1@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> >
> > Hi Haiyue,
> >
> > This statement is confusing to me "But the flow setting is the ice PF
> > AdminQ message, so the DCF shares most of ice PMD flow control." What
> do you mean by "flow setting"?
>   It is DPDP rte_flow API calling.
> 
> > The way I understand DCF
> > working is that there is a trusted VF (the DCF) that is setting switch
> > rules for other VFs on the same PF. The mechanism for doing that is
> > the DCF sends a virtchnl message to the Linux kernel driver PF to
> 
> DCF needs: 1). virtchnl message is handles by 'dpdk/drivers/common/iavf/',
> which is the original iavf base code.
> 
> > add/delete a switch rule. None of this requires the ice PMD as far as
> > I can tell. This seems like a
> 
> DCF needs: 2). add/delete a switch rule.
>                rte_flow API -->
> 'dpdk/drivers/net/ice/ice_switch_filter.c/ice_generic_flow.c' (ice flow
> framework)
>                      |
>                                  `--> 'dpdk/drivers/net/ice/base' ...
> 
> So, put it under the ice PMD is the best way.
> 
> > driver just like the iavf driver; the iavf driver is separate from the
> > ice PMD and it seems like DCF should also be separate.
> >
> > Paul
> >
> > > -----Original Message-----
> > > From: Wang, Haiyue <haiyue.wang@intel.com>
> > > Sent: Friday, March 13, 2020 9:25 AM
> > > To: Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>;
> > > dev@dpdk.org; Ye, Xiaolong <xiaolong.ye@intel.com>; Zhang, Qi Z
> > > <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Xing,
> > > Beilei <beilei.xing@intel.com>
> > > Cc: Zhao1, Wei <wei.zhao1@intel.com>
> > > Subject: RE: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> > >
> > > Hi Paul,
> > >
> > > Yes, it's VF (VF hardware initialization like virtchnl). But the
> > > flow setting is the ice PF AdminQ message, so the DCF shares most of ice
> PMD flow control.
> > >
> > > BR,
> > > Haiyue
> > >
> > > > -----Original Message-----
> > > > From: Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>
> > > > Sent: Saturday, March 14, 2020 00:19
> > > > To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Ye,
> > > > Xiaolong <xiaolong.ye@intel.com>; Zhang, Qi Z
> > > > <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>;
> > > > Xing, Beilei <beilei.xing@intel.com>
> > > > Cc: Zhao1, Wei <wei.zhao1@intel.com>; Wang, Haiyue
> > > > <haiyue.wang@intel.com>
> > > > Subject: RE: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> > > >
> > > > I'm confused. Shouldn't the DCF be a separate driver since it is a
> > > > VF, not part of a PF? You are starting to combine PF/VF code and
> > > > I'm not sure if
> > > that is the correct way to go.
> > > >
> > > > Paul
> > > >
> > > > > -----Original Message-----
> > > > > From: dev <dev-bounces@dpdk.org> On Behalf Of Haiyue Wang
> > > > > Sent: Monday, March 9, 2020 11:50 PM
> > > > > To: dev@dpdk.org; Ye, Xiaolong <xiaolong.ye@intel.com>; Zhang,
> > > > > Qi Z <qi.z.zhang@intel.com>; Yang, Qiming
> > > > > <qiming.yang@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> > > > > Cc: Zhao1, Wei <wei.zhao1@intel.com>; Wang, Haiyue
> > > > > <haiyue.wang@intel.com>
> > > > > Subject: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> > > > >
> > > > > A DCF (Device Config Function) based approach is proposed where
> > > > > a device bound to the device's VF0 can act as a sole controlling
> > > > > entity to exercise advance functionality (such as switch, ACL)
> > > > > for rest of
> > > the VFs.
> > > > >
> > > > > The DCF works as a standalone PMD to support this function,
> > > > > which shares the ice PMD flow control core function and the iavf
> > > > > virtchnl mailbox core module.
> > > > >
> > > > > This patchset is based on:
> > > > > [1] https://patchwork.dpdk.org/cover/66417/ : update ice base
> > > > > code [2] https://patchwork.dpdk.org/cover/66472/ : iavf share
> > > > > code update
> > > > >
> > > > > Depends-on: series-8843
> > > > > Depends-on: series-8855
> > > > >
> > > > > v2:
> > > > >    1. update the iavf patchset link.
> > > > >    2. split more patches for making this work be more understandable
> > > > >    3. fix the log function usage, devargs checking from v1.
> > > > >
> > > > > Haiyue Wang (7):
> > > > >   net/iavf: stop the PCI probe in DCF mode
> > > > >   net/ice: add the DCF hardware initialization
> > > > >   net/ice: initiate to acquire the DCF capability
> > > > >   net/ice: handle the AdminQ command by DCF
> > > > >   net/ice: export the DDP definition symbols
> > > > >   net/ice: handle the PF initialization by DCF
> > > > >   net/ice: get the VF hardware index in DCF
> > > > >
> > > > >  doc/guides/nics/ice.rst                |  47 ++
> > > > >  doc/guides/nics/img/ice_dcf.png        | Bin 0 -> 39168 bytes
> > > > >  doc/guides/rel_notes/release_20_05.rst |   5 +
> > > > >  drivers/common/Makefile                |   1 +
> > > > >  drivers/net/iavf/iavf_ethdev.c         |  43 ++
> > > > >  drivers/net/ice/Makefile               |   6 +
> > > > >  drivers/net/ice/ice_dcf.c              | 651
> +++++++++++++++++++++++++
> > > > >  drivers/net/ice/ice_dcf.h              |  61 +++
> > > > >  drivers/net/ice/ice_dcf_ethdev.c       | 321 ++++++++++++
> > > > >  drivers/net/ice/ice_dcf_ethdev.h       |  33 ++
> > > > >  drivers/net/ice/ice_dcf_parent.c       | 344 +++++++++++++
> > > > >  drivers/net/ice/ice_ethdev.c           |   9 +-
> > > > >  drivers/net/ice/ice_ethdev.h           |   8 +
> > > > >  drivers/net/ice/meson.build            |   8 +-
> > > > >  mk/rte.app.mk                          |   1 +
> > > > >  15 files changed, 1528 insertions(+), 10 deletions(-)  create
> > > > > mode
> > > > > 100644 doc/guides/nics/img/ice_dcf.png  create mode 100644
> > > > > drivers/net/ice/ice_dcf.c  create mode 100644
> > > > > drivers/net/ice/ice_dcf.h create mode 100644
> > > > > drivers/net/ice/ice_dcf_ethdev.c  create mode 100644
> > > > > drivers/net/ice/ice_dcf_ethdev.h  create mode 100644
> > > > > drivers/net/ice/ice_dcf_parent.c
> > > > >
> > > > > --
> > > > > 2.25.1
> > > >
> > >
> >
>
Wang, Haiyue March 14, 2020, 1:57 a.m. UTC | #6 
> -----Original Message-----
> From: Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>
> Sent: Saturday, March 14, 2020 01:48
> To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Ye, Xiaolong <xiaolong.ye@intel.com>; Zhang,
> Qi Z <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Cc: Zhao1, Wei <wei.zhao1@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> 
> OK, I looked at the code further and it seems like what you are doing is you are using the iavf PCI
> device ID instead of the PF device ID when the user says cap=dcf. This doesn't seem like the right
> thing to do.

No, how can you get the PF device ID from the VF hardware ? ;-)

> Why not modify the iavf code to support being the DCF? Or create a new PMD? You are
> calling iavf functions from within the ice PMD which seems wrong to me.

No, you still can't get my point about the ice rte_flow framework. Please see Wei's next patch 

https://patchwork.dpdk.org/patch/66621/ net/ice: enable switch flow on DCF

> 
> Paul
> 
> > -----Original Message-----
> > From: Wang, Haiyue <haiyue.wang@intel.com>
> > Sent: Friday, March 13, 2020 10:05 AM
> > To: Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>; dev@dpdk.org; Ye,
> > Xiaolong <xiaolong.ye@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> > Yang, Qiming <qiming.yang@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> > Cc: Zhao1, Wei <wei.zhao1@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> >
> > Hi Paul,
> >
> > > -----Original Message-----
> > > From: Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>
> > > Sent: Saturday, March 14, 2020 00:50
> > > To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Ye, Xiaolong
> > > <xiaolong.ye@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Yang,
> > > Qiming <qiming.yang@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> > > Cc: Zhao1, Wei <wei.zhao1@intel.com>
> > > Subject: RE: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> > >
> > > Hi Haiyue,
> > >
> > > This statement is confusing to me "But the flow setting is the ice PF
> > > AdminQ message, so the DCF shares most of ice PMD flow control." What
> > do you mean by "flow setting"?
> >   It is DPDP rte_flow API calling.
> >
> > > The way I understand DCF
> > > working is that there is a trusted VF (the DCF) that is setting switch
> > > rules for other VFs on the same PF. The mechanism for doing that is
> > > the DCF sends a virtchnl message to the Linux kernel driver PF to
> >
> > DCF needs: 1). virtchnl message is handles by 'dpdk/drivers/common/iavf/',
> > which is the original iavf base code.
> >
> > > add/delete a switch rule. None of this requires the ice PMD as far as
> > > I can tell. This seems like a
> >
> > DCF needs: 2). add/delete a switch rule.
> >                rte_flow API -->
> > 'dpdk/drivers/net/ice/ice_switch_filter.c/ice_generic_flow.c' (ice flow
> > framework)
> >                      |
> >                                  `--> 'dpdk/drivers/net/ice/base' ...
> >
> > So, put it under the ice PMD is the best way.
> >
> > > driver just like the iavf driver; the iavf driver is separate from the
> > > ice PMD and it seems like DCF should also be separate.
> > >
> > > Paul
> > >
> > > > -----Original Message-----
> > > > From: Wang, Haiyue <haiyue.wang@intel.com>
> > > > Sent: Friday, March 13, 2020 9:25 AM
> > > > To: Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>;
> > > > dev@dpdk.org; Ye, Xiaolong <xiaolong.ye@intel.com>; Zhang, Qi Z
> > > > <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Xing,
> > > > Beilei <beilei.xing@intel.com>
> > > > Cc: Zhao1, Wei <wei.zhao1@intel.com>
> > > > Subject: RE: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> > > >
> > > > Hi Paul,
> > > >
> > > > Yes, it's VF (VF hardware initialization like virtchnl). But the
> > > > flow setting is the ice PF AdminQ message, so the DCF shares most of ice
> > PMD flow control.
> > > >
> > > > BR,
> > > > Haiyue
> > > >
> > > > > -----Original Message-----
> > > > > From: Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>
> > > > > Sent: Saturday, March 14, 2020 00:19
> > > > > To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Ye,
> > > > > Xiaolong <xiaolong.ye@intel.com>; Zhang, Qi Z
> > > > > <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>;
> > > > > Xing, Beilei <beilei.xing@intel.com>
> > > > > Cc: Zhao1, Wei <wei.zhao1@intel.com>; Wang, Haiyue
> > > > > <haiyue.wang@intel.com>
> > > > > Subject: RE: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> > > > >
> > > > > I'm confused. Shouldn't the DCF be a separate driver since it is a
> > > > > VF, not part of a PF? You are starting to combine PF/VF code and
> > > > > I'm not sure if
> > > > that is the correct way to go.
> > > > >
> > > > > Paul
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: dev <dev-bounces@dpdk.org> On Behalf Of Haiyue Wang
> > > > > > Sent: Monday, March 9, 2020 11:50 PM
> > > > > > To: dev@dpdk.org; Ye, Xiaolong <xiaolong.ye@intel.com>; Zhang,
> > > > > > Qi Z <qi.z.zhang@intel.com>; Yang, Qiming
> > > > > > <qiming.yang@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> > > > > > Cc: Zhao1, Wei <wei.zhao1@intel.com>; Wang, Haiyue
> > > > > > <haiyue.wang@intel.com>
> > > > > > Subject: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> > > > > >
> > > > > > A DCF (Device Config Function) based approach is proposed where
> > > > > > a device bound to the device's VF0 can act as a sole controlling
> > > > > > entity to exercise advance functionality (such as switch, ACL)
> > > > > > for rest of
> > > > the VFs.
> > > > > >
> > > > > > The DCF works as a standalone PMD to support this function,
> > > > > > which shares the ice PMD flow control core function and the iavf
> > > > > > virtchnl mailbox core module.
> > > > > >
> > > > > > This patchset is based on:
> > > > > > [1] https://patchwork.dpdk.org/cover/66417/ : update ice base
> > > > > > code [2] https://patchwork.dpdk.org/cover/66472/ : iavf share
> > > > > > code update
> > > > > >
> > > > > > Depends-on: series-8843
> > > > > > Depends-on: series-8855
> > > > > >
> > > > > > v2:
> > > > > >    1. update the iavf patchset link.
> > > > > >    2. split more patches for making this work be more understandable
> > > > > >    3. fix the log function usage, devargs checking from v1.
> > > > > >
> > > > > > Haiyue Wang (7):
> > > > > >   net/iavf: stop the PCI probe in DCF mode
> > > > > >   net/ice: add the DCF hardware initialization
> > > > > >   net/ice: initiate to acquire the DCF capability
> > > > > >   net/ice: handle the AdminQ command by DCF
> > > > > >   net/ice: export the DDP definition symbols
> > > > > >   net/ice: handle the PF initialization by DCF
> > > > > >   net/ice: get the VF hardware index in DCF
> > > > > >
> > > > > >  doc/guides/nics/ice.rst                |  47 ++
> > > > > >  doc/guides/nics/img/ice_dcf.png        | Bin 0 -> 39168 bytes
> > > > > >  doc/guides/rel_notes/release_20_05.rst |   5 +
> > > > > >  drivers/common/Makefile                |   1 +
> > > > > >  drivers/net/iavf/iavf_ethdev.c         |  43 ++
> > > > > >  drivers/net/ice/Makefile               |   6 +
> > > > > >  drivers/net/ice/ice_dcf.c              | 651
> > +++++++++++++++++++++++++
> > > > > >  drivers/net/ice/ice_dcf.h              |  61 +++
> > > > > >  drivers/net/ice/ice_dcf_ethdev.c       | 321 ++++++++++++
> > > > > >  drivers/net/ice/ice_dcf_ethdev.h       |  33 ++
> > > > > >  drivers/net/ice/ice_dcf_parent.c       | 344 +++++++++++++
> > > > > >  drivers/net/ice/ice_ethdev.c           |   9 +-
> > > > > >  drivers/net/ice/ice_ethdev.h           |   8 +
> > > > > >  drivers/net/ice/meson.build            |   8 +-
> > > > > >  mk/rte.app.mk                          |   1 +
> > > > > >  15 files changed, 1528 insertions(+), 10 deletions(-)  create
> > > > > > mode
> > > > > > 100644 doc/guides/nics/img/ice_dcf.png  create mode 100644
> > > > > > drivers/net/ice/ice_dcf.c  create mode 100644
> > > > > > drivers/net/ice/ice_dcf.h create mode 100644
> > > > > > drivers/net/ice/ice_dcf_ethdev.c  create mode 100644
> > > > > > drivers/net/ice/ice_dcf_ethdev.h  create mode 100644
> > > > > > drivers/net/ice/ice_dcf_parent.c
> > > > > >
> > > > > > --
> > > > > > 2.25.1
> > > > >
> > > >
> > >
> >
>
Qi Zhang March 15, 2020, 1:49 a.m. UTC | #7 
Hi Paul:

> -----Original Message-----
> From: Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>
> Sent: Saturday, March 14, 2020 12:19 AM
> To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Ye, Xiaolong
> <xiaolong.ye@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Cc: Zhao1, Wei <wei.zhao1@intel.com>; Wang, Haiyue
> <haiyue.wang@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> 
> I'm confused. Shouldn't the DCF be a separate driver since it is a VF, not part
> of a PF? You are starting to combine PF/VF code and I'm not sure if that is
> the correct way to go.

From DPDK's view, DCF is NOT a VF driver, actually it behaves like a PF driver with control path only and it share most exist PF driver's API implementation , 
That's why we combined it with exist PF driver, so when the module driver/net/ice is compiled into a library, it can be probed as a PF driver or DCF base on device ID at runtime. 
And the is not special in DPDK, for example, the module in driver/net/i40e can be probed as i40e pf driver or i40e vf driver.
DCF take SR-IOV's mailbox as a message channel to communicate with the backend kernel driver, so iavf virtual channel is reused here, so we also need to link iavf share code.

And I agree, its always better if we can separate 2 different things while decouple all the common code into a share library to avoid duplicate code copy (just like we did on iavf share code)
But in this case, a lot of code refectory is required , so far we just take the simple way, we may improve this in future. 
 
Thanks
Qi

> 
> Paul
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Haiyue Wang
> > Sent: Monday, March 9, 2020 11:50 PM
> > To: dev@dpdk.org; Ye, Xiaolong <xiaolong.ye@intel.com>; Zhang, Qi Z
> > <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Xing,
> > Beilei <beilei.xing@intel.com>
> > Cc: Zhao1, Wei <wei.zhao1@intel.com>; Wang, Haiyue
> > <haiyue.wang@intel.com>
> > Subject: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> >
> > A DCF (Device Config Function) based approach is proposed where a
> > device bound to the device's VF0 can act as a sole controlling entity
> > to exercise advance functionality (such as switch, ACL) for rest of the VFs.
> >
> > The DCF works as a standalone PMD to support this function, which
> > shares the ice PMD flow control core function and the iavf virtchnl
> > mailbox core module.
> >
> > This patchset is based on:
> > [1] https://patchwork.dpdk.org/cover/66417/ : update ice base code [2]
> > https://patchwork.dpdk.org/cover/66472/ : iavf share code update
> >
> > Depends-on: series-8843
> > Depends-on: series-8855
> >
> > v2:
> >    1. update the iavf patchset link.
> >    2. split more patches for making this work be more understandable
> >    3. fix the log function usage, devargs checking from v1.
> >
> > Haiyue Wang (7):
> >   net/iavf: stop the PCI probe in DCF mode
> >   net/ice: add the DCF hardware initialization
> >   net/ice: initiate to acquire the DCF capability
> >   net/ice: handle the AdminQ command by DCF
> >   net/ice: export the DDP definition symbols
> >   net/ice: handle the PF initialization by DCF
> >   net/ice: get the VF hardware index in DCF
> >
> >  doc/guides/nics/ice.rst                |  47 ++
> >  doc/guides/nics/img/ice_dcf.png        | Bin 0 -> 39168 bytes
> >  doc/guides/rel_notes/release_20_05.rst |   5 +
> >  drivers/common/Makefile                |   1 +
> >  drivers/net/iavf/iavf_ethdev.c         |  43 ++
> >  drivers/net/ice/Makefile               |   6 +
> >  drivers/net/ice/ice_dcf.c              | 651
> +++++++++++++++++++++++++
> >  drivers/net/ice/ice_dcf.h              |  61 +++
> >  drivers/net/ice/ice_dcf_ethdev.c       | 321 ++++++++++++
> >  drivers/net/ice/ice_dcf_ethdev.h       |  33 ++
> >  drivers/net/ice/ice_dcf_parent.c       | 344 +++++++++++++
> >  drivers/net/ice/ice_ethdev.c           |   9 +-
> >  drivers/net/ice/ice_ethdev.h           |   8 +
> >  drivers/net/ice/meson.build            |   8 +-
> >  mk/rte.app.mk                          |   1 +
> >  15 files changed, 1528 insertions(+), 10 deletions(-)  create mode
> > 100644 doc/guides/nics/img/ice_dcf.png  create mode 100644
> > drivers/net/ice/ice_dcf.c  create mode 100644
> > drivers/net/ice/ice_dcf.h create mode 100644
> > drivers/net/ice/ice_dcf_ethdev.c  create mode 100644
> > drivers/net/ice/ice_dcf_ethdev.h  create mode 100644
> > drivers/net/ice/ice_dcf_parent.c
> >
> > --
> > 2.25.1
>
Stillwell Jr, Paul M March 16, 2020, 6:54 p.m. UTC | #8 
See my responses inline.

Paul

> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: Saturday, March 14, 2020 6:50 PM
> To: Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>; Wang, Haiyue
> <haiyue.wang@intel.com>; dev@dpdk.org; Ye, Xiaolong
> <xiaolong.ye@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Xing,
> Beilei <beilei.xing@intel.com>
> Cc: Zhao1, Wei <wei.zhao1@intel.com>; Wang, Haiyue
> <haiyue.wang@intel.com>; Liang, Cunming <cunming.liang@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> 
> Hi Paul:
> 
> > -----Original Message-----
> > From: Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>
> > Sent: Saturday, March 14, 2020 12:19 AM
> > To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Ye, Xiaolong
> > <xiaolong.ye@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Yang,
> > Qiming <qiming.yang@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> > Cc: Zhao1, Wei <wei.zhao1@intel.com>; Wang, Haiyue
> > <haiyue.wang@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> >
> > I'm confused. Shouldn't the DCF be a separate driver since it is a VF,
> > not part of a PF? You are starting to combine PF/VF code and I'm not
> > sure if that is the correct way to go.
> 
> From DPDK's view, DCF is NOT a VF driver, actually it behaves like a PF driver
> with control path only and it share most exist PF driver's API implementation

It *is* a VF. The only way this will work is if the VF has the trust flag set and then it can become the DCF. You can't set the trust flag on a PF I don't think. That's the only way this will work because the kernel driver code has checks on the admin queue messages to see if the message is from a VF. If it's not a message from a specific VF, then the message will get rejected.

> , That's why we combined it with exist PF driver, so when the module
> driver/net/ice is compiled into a library, it can be probed as a PF driver or DCF
> base on device ID at runtime.
> And the is not special in DPDK, for example, the module in driver/net/i40e
> can be probed as i40e pf driver or i40e vf driver.
> DCF take SR-IOV's mailbox as a message channel to communicate with the
> backend kernel driver, so iavf virtual channel is reused here, so we also need
> to link iavf share code.
> 
> And I agree, its always better if we can separate 2 different things while
> decouple all the common code into a share library to avoid duplicate code
> copy (just like we did on iavf share code) But in this case, a lot of code
> refectory is required , so far we just take the simple way, we may improve
> this in future.
> 
> Thanks
> Qi
> 
> >
> > Paul
> >
> > > -----Original Message-----
> > > From: dev <dev-bounces@dpdk.org> On Behalf Of Haiyue Wang
> > > Sent: Monday, March 9, 2020 11:50 PM
> > > To: dev@dpdk.org; Ye, Xiaolong <xiaolong.ye@intel.com>; Zhang, Qi Z
> > > <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Xing,
> > > Beilei <beilei.xing@intel.com>
> > > Cc: Zhao1, Wei <wei.zhao1@intel.com>; Wang, Haiyue
> > > <haiyue.wang@intel.com>
> > > Subject: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> > >
> > > A DCF (Device Config Function) based approach is proposed where a
> > > device bound to the device's VF0 can act as a sole controlling
> > > entity to exercise advance functionality (such as switch, ACL) for rest of
> the VFs.
> > >
> > > The DCF works as a standalone PMD to support this function, which
> > > shares the ice PMD flow control core function and the iavf virtchnl
> > > mailbox core module.
> > >
> > > This patchset is based on:
> > > [1] https://patchwork.dpdk.org/cover/66417/ : update ice base code
> > > [2] https://patchwork.dpdk.org/cover/66472/ : iavf share code update
> > >
> > > Depends-on: series-8843
> > > Depends-on: series-8855
> > >
> > > v2:
> > >    1. update the iavf patchset link.
> > >    2. split more patches for making this work be more understandable
> > >    3. fix the log function usage, devargs checking from v1.
> > >
> > > Haiyue Wang (7):
> > >   net/iavf: stop the PCI probe in DCF mode
> > >   net/ice: add the DCF hardware initialization
> > >   net/ice: initiate to acquire the DCF capability
> > >   net/ice: handle the AdminQ command by DCF
> > >   net/ice: export the DDP definition symbols
> > >   net/ice: handle the PF initialization by DCF
> > >   net/ice: get the VF hardware index in DCF
> > >
> > >  doc/guides/nics/ice.rst                |  47 ++
> > >  doc/guides/nics/img/ice_dcf.png        | Bin 0 -> 39168 bytes
> > >  doc/guides/rel_notes/release_20_05.rst |   5 +
> > >  drivers/common/Makefile                |   1 +
> > >  drivers/net/iavf/iavf_ethdev.c         |  43 ++
> > >  drivers/net/ice/Makefile               |   6 +
> > >  drivers/net/ice/ice_dcf.c              | 651
> > +++++++++++++++++++++++++
> > >  drivers/net/ice/ice_dcf.h              |  61 +++
> > >  drivers/net/ice/ice_dcf_ethdev.c       | 321 ++++++++++++
> > >  drivers/net/ice/ice_dcf_ethdev.h       |  33 ++
> > >  drivers/net/ice/ice_dcf_parent.c       | 344 +++++++++++++
> > >  drivers/net/ice/ice_ethdev.c           |   9 +-
> > >  drivers/net/ice/ice_ethdev.h           |   8 +
> > >  drivers/net/ice/meson.build            |   8 +-
> > >  mk/rte.app.mk                          |   1 +
> > >  15 files changed, 1528 insertions(+), 10 deletions(-)  create mode
> > > 100644 doc/guides/nics/img/ice_dcf.png  create mode 100644
> > > drivers/net/ice/ice_dcf.c  create mode 100644
> > > drivers/net/ice/ice_dcf.h create mode 100644
> > > drivers/net/ice/ice_dcf_ethdev.c  create mode 100644
> > > drivers/net/ice/ice_dcf_ethdev.h  create mode 100644
> > > drivers/net/ice/ice_dcf_parent.c
> > >
> > > --
> > > 2.25.1
> >
>
Wang, Haiyue March 17, 2020, 2:35 a.m. UTC | #9 
> -----Original Message-----
> From: Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>
> Sent: Tuesday, March 17, 2020 02:55
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Ye,
> Xiaolong <xiaolong.ye@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>
> Cc: Zhao1, Wei <wei.zhao1@intel.com>; Wang, Haiyue <haiyue.wang@intel.com>; Liang, Cunming
> <cunming.liang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> 
> See my responses inline.
> 
> Paul
> 
> > -----Original Message-----
> > From: Zhang, Qi Z <qi.z.zhang@intel.com>
> > Sent: Saturday, March 14, 2020 6:50 PM
> > To: Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>; Wang, Haiyue
> > <haiyue.wang@intel.com>; dev@dpdk.org; Ye, Xiaolong
> > <xiaolong.ye@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Xing,
> > Beilei <beilei.xing@intel.com>
> > Cc: Zhao1, Wei <wei.zhao1@intel.com>; Wang, Haiyue
> > <haiyue.wang@intel.com>; Liang, Cunming <cunming.liang@intel.com>; Wu,
> > Jingjing <jingjing.wu@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> >
> > Hi Paul:
> >
> > > -----Original Message-----
> > > From: Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>
> > > Sent: Saturday, March 14, 2020 12:19 AM
> > > To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Ye, Xiaolong
> > > <xiaolong.ye@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Yang,
> > > Qiming <qiming.yang@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> > > Cc: Zhao1, Wei <wei.zhao1@intel.com>; Wang, Haiyue
> > > <haiyue.wang@intel.com>
> > > Subject: RE: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> > >
> > > I'm confused. Shouldn't the DCF be a separate driver since it is a VF,
> > > not part of a PF? You are starting to combine PF/VF code and I'm not
> > > sure if that is the correct way to go.
> >
> > From DPDK's view, DCF is NOT a VF driver, actually it behaves like a PF driver
> > with control path only and it share most exist PF driver's API implementation
> 
> It *is* a VF. The only way this will work is if the VF has the trust flag set and then it can become
> the DCF. You can't set the trust flag on a PF I don't think. That's the only way this will work
> because the kernel driver code has checks on the admin queue messages to see if the message is from a
> VF. If it's not a message from a specific VF, then the message will get rejected.
> 

This is absolutely right, and this is added into the guide about how to enable DCF. ;-)

And one important thing is that : VF with DCF capability is only bound to ice/kernel
driver, so we can't add it into the generic iavf VF PMD, since it works on i40e/ice etc.
And the main task about DCF is flow control, the AdminQ is implemented in ice PF PMD, and
it just uses these AdminQ API to runs on VF hardware, not ice PF hardware.


> > , That's why we combined it with exist PF driver, so when the module
> > driver/net/ice is compiled into a library, it can be probed as a PF driver or DCF
> > base on device ID at runtime.
> > And the is not special in DPDK, for example, the module in driver/net/i40e
> > can be probed as i40e pf driver or i40e vf driver.
> > DCF take SR-IOV's mailbox as a message channel to communicate with the
> > backend kernel driver, so iavf virtual channel is reused here, so we also need
> > to link iavf share code.
> >
> > And I agree, its always better if we can separate 2 different things while
> > decouple all the common code into a share library to avoid duplicate code
> > copy (just like we did on iavf share code) But in this case, a lot of code
> > refectory is required , so far we just take the simple way, we may improve
> > this in future.
> >
> > Thanks
> > Qi
> >
> > >
> > > Paul
> > >
> > > > -----Original Message-----
> > > > From: dev <dev-bounces@dpdk.org> On Behalf Of Haiyue Wang
> > > > Sent: Monday, March 9, 2020 11:50 PM
> > > > To: dev@dpdk.org; Ye, Xiaolong <xiaolong.ye@intel.com>; Zhang, Qi Z
> > > > <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Xing,
> > > > Beilei <beilei.xing@intel.com>
> > > > Cc: Zhao1, Wei <wei.zhao1@intel.com>; Wang, Haiyue
> > > > <haiyue.wang@intel.com>
> > > > Subject: [dpdk-dev] [PATCH v2 0/7] add Intel DCF PMD support
> > > >
> > > > A DCF (Device Config Function) based approach is proposed where a
> > > > device bound to the device's VF0 can act as a sole controlling
> > > > entity to exercise advance functionality (such as switch, ACL) for rest of
> > the VFs.
> > > >
> > > > The DCF works as a standalone PMD to support this function, which
> > > > shares the ice PMD flow control core function and the iavf virtchnl
> > > > mailbox core module.
> > > >
> > > > This patchset is based on:
> > > > [1] https://patchwork.dpdk.org/cover/66417/ : update ice base code
> > > > [2] https://patchwork.dpdk.org/cover/66472/ : iavf share code update
> > > >
> > > > Depends-on: series-8843
> > > > Depends-on: series-8855
> > > >
> > > > v2:
> > > >    1. update the iavf patchset link.
> > > >    2. split more patches for making this work be more understandable
> > > >    3. fix the log function usage, devargs checking from v1.
> > > >
> > > > Haiyue Wang (7):
> > > >   net/iavf: stop the PCI probe in DCF mode
> > > >   net/ice: add the DCF hardware initialization
> > > >   net/ice: initiate to acquire the DCF capability
> > > >   net/ice: handle the AdminQ command by DCF
> > > >   net/ice: export the DDP definition symbols
> > > >   net/ice: handle the PF initialization by DCF
> > > >   net/ice: get the VF hardware index in DCF
> > > >
> > > >  doc/guides/nics/ice.rst                |  47 ++
> > > >  doc/guides/nics/img/ice_dcf.png        | Bin 0 -> 39168 bytes
> > > >  doc/guides/rel_notes/release_20_05.rst |   5 +
> > > >  drivers/common/Makefile                |   1 +
> > > >  drivers/net/iavf/iavf_ethdev.c         |  43 ++
> > > >  drivers/net/ice/Makefile               |   6 +
> > > >  drivers/net/ice/ice_dcf.c              | 651
> > > +++++++++++++++++++++++++
> > > >  drivers/net/ice/ice_dcf.h              |  61 +++
> > > >  drivers/net/ice/ice_dcf_ethdev.c       | 321 ++++++++++++
> > > >  drivers/net/ice/ice_dcf_ethdev.h       |  33 ++
> > > >  drivers/net/ice/ice_dcf_parent.c       | 344 +++++++++++++
> > > >  drivers/net/ice/ice_ethdev.c           |   9 +-
> > > >  drivers/net/ice/ice_ethdev.h           |   8 +
> > > >  drivers/net/ice/meson.build            |   8 +-
> > > >  mk/rte.app.mk                          |   1 +
> > > >  15 files changed, 1528 insertions(+), 10 deletions(-)  create mode
> > > > 100644 doc/guides/nics/img/ice_dcf.png  create mode 100644
> > > > drivers/net/ice/ice_dcf.c  create mode 100644
> > > > drivers/net/ice/ice_dcf.h create mode 100644
> > > > drivers/net/ice/ice_dcf_ethdev.c  create mode 100644
> > > > drivers/net/ice/ice_dcf_ethdev.h  create mode 100644
> > > > drivers/net/ice/ice_dcf_parent.c
> > > >
> > > > --
> > > > 2.25.1
> > >
> >
>