| Message ID | 20230508194326.482-1-emil.s.tantilov@intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Introduce Intel IDPF driver | expand |
On Mon, May 08, 2023 at 12:43:11PM -0700, Emil Tantilov wrote: > This patch series introduces the Intel Infrastructure Data Path Function > (IDPF) driver. It is used for both physical and virtual functions. Except > for some of the device operations the rest of the functionality is the > same for both PF and VF. IDPF uses virtchnl version2 opcodes and > structures defined in the virtchnl2 header file which helps the driver > to learn the capabilities and register offsets from the device > Control Plane (CP) instead of assuming the default values. So, is this for merge in the next cycle? Should this be an RFC rather? It seems unlikely that the IDPF specification will be finalized by that time - how are you going to handle any specification changes? > The format of the series follows the driver init flow to interface open. > To start with, probe gets called and kicks off the driver initialization > by spawning the 'vc_event_task' work queue which in turn calls the > 'hard reset' function. As part of that, the mailbox is initialized which > is used to send/receive the virtchnl messages to/from the CP. Once that is > done, 'core init' kicks in which requests all the required global resources > from the CP and spawns the 'init_task' work queue to create the vports. > > Based on the capability information received, the driver creates the said > number of vports (one or many) where each vport is associated to a netdev. > Also, each vport has its own resources such as queues, vectors etc. > >From there, rest of the netdev_ops and data path are added. > > IDPF implements both single queue which is traditional queueing model > as well as split queue model. In split queue model, it uses separate queue > for both completion descriptors and buffers which helps to implement > out-of-order completions. It also helps to implement asymmetric queues, > for example multiple RX completion queues can be processed by a single > RX buffer queue and multiple TX buffer queues can be processed by a > single TX completion queue. In single queue model, same queue is used > for both descriptor completions as well as buffer completions. It also > supports features such as generic checksum offload, generic receive > offload (hardware GRO) etc. > > v3 --> v4: link [3] > (patch 1): > * cleanups in virtchnl2 including redundant error codes, naming and > whitespace > (patch 3): > * removed "__" prefix from names of adapter and vport flags, converted > comments to kernel-doc style > * renamed error code variable names in controlq to be more consistent > with rest of the code > * removed Control Plane specific opcodes and changed "peer" type comments > to CP > * replaced managed dma calls with their non-managed equivalent > (patch 4): > * added additional info to some error messages on init to aid in debug > * removed unnecessary zero-init before loop and zeroing memcpy after > kzalloc() > * corrected wording of comment in idpf_wait_for_event() s/wake up/woken/ > * replaced managed dma calls with their non-managed equivalent > > [3] https://lore.kernel.org/netdev/20230427020917.12029-1-emil.s.tantilov@intel.com/ > > v2 --> v3: link [2] > * converted virtchnl2 defines to enums > * fixed comment style in virtchnl2 to follow kernel-doc format > * removed empty lines between end of structs and size check macro > checkpatch will mark these instances as CHECK > * cleaned up unused Rx descriptor structs and related bits in virtchnl2 > * converted Rx descriptor bit offsets into bitmasks to better align with > the use of GENMASK and FIELD_GET > * added device ids to pci_tbl from the start > * consolidated common probe and remove functions into idpf_probe() and > idpf_remove() respectively > * removed needless adapter NULL checks > * removed devm_kzalloc() in favor of kzalloc(), including kfree in > error and exit code path > * replaced instances of kcalloc() calls where either size parameter was > 1 with kzalloc(), reported by smatch > * used kmemdup() in some instances reported by coccicheck > * added explicit error code and comment explaining the condition for > the exit to address warning by smatch > * moved build support to the last patch > > [2] https://lore.kernel.org/netdev/20230411011354.2619359-1-pavan.kumar.linga@intel.com/ > > v1 --> v2: link [1] > * removed the OASIS reference in the commit message to make it clear > that this is an Intel vendor specific driver > * fixed misspells > * used comment starter "/**" for struct and definition headers in > virtchnl header files > * removed AVF reference > * renamed APF reference to IDPF > * added a comment to explain the reason for 'no flex field' at the end of > virtchnl2_get_ptype_info struct > * removed 'key[1]' in virtchnl2_rss_key struct as it is not used > * set VIRTCHNL2_RXDID_2_FLEX_SQ_NIC to VIRTCHNL2_RXDID_2_FLEX_SPLITQ > instead of assigning the same value > * cleanup unnecessary NULL assignment to the rx_buf skb pointer since > it is not used in splitq model > * added comments to clarify the generation bit usage in splitq model > * introduced 'reuse_bias' in the page_info structure and make use of it > in the hot path > * fixed RCT format in idpf_rx_construct_skb > * report SPEED_UNKNOWN and DUPLEX_UNKNOWN when the link is down > * fixed -Wframe-larger-than warning reported by lkp bot in > idpf_vport_queue_ids_init > * updated the documentation in idpf.rst to fix LKP bot warning > > [1] https://lore.kernel.org/netdev/20230329140404.1647925-1-pavan.kumar.linga@intel.com/ > > Alan Brady (4): > idpf: configure resources for TX queues > idpf: configure resources for RX queues > idpf: add RX splitq napi poll support > idpf: add ethtool callbacks > > Joshua Hay (5): > idpf: add controlq init and reset checks > idpf: add splitq start_xmit > idpf: add TX splitq napi poll support > idpf: add singleq start_xmit and napi poll > idpf: configure SRIOV and add other ndo_ops > > Pavan Kumar Linga (5): > virtchnl: add virtchnl version 2 ops > idpf: add core init and interrupt request > idpf: add create vport and netdev configuration > idpf: continue expanding init task > idpf: initialize interrupts and enable vport > > Phani Burra (1): > idpf: add module register and probe functionality > > .../device_drivers/ethernet/intel/idpf.rst | 162 + > drivers/net/ethernet/intel/Kconfig | 10 + > drivers/net/ethernet/intel/Makefile | 1 + > drivers/net/ethernet/intel/idpf/Makefile | 18 + > drivers/net/ethernet/intel/idpf/idpf.h | 752 +++ > .../net/ethernet/intel/idpf/idpf_controlq.c | 641 +++ > .../net/ethernet/intel/idpf/idpf_controlq.h | 131 + > .../ethernet/intel/idpf/idpf_controlq_api.h | 169 + > .../ethernet/intel/idpf/idpf_controlq_setup.c | 175 + > drivers/net/ethernet/intel/idpf/idpf_dev.c | 165 + > drivers/net/ethernet/intel/idpf/idpf_devids.h | 10 + > .../net/ethernet/intel/idpf/idpf_ethtool.c | 1330 +++++ > .../ethernet/intel/idpf/idpf_lan_pf_regs.h | 124 + > .../net/ethernet/intel/idpf/idpf_lan_txrx.h | 293 + > .../ethernet/intel/idpf/idpf_lan_vf_regs.h | 128 + > drivers/net/ethernet/intel/idpf/idpf_lib.c | 2349 ++++++++ > drivers/net/ethernet/intel/idpf/idpf_main.c | 266 + > drivers/net/ethernet/intel/idpf/idpf_mem.h | 20 + > .../ethernet/intel/idpf/idpf_singleq_txrx.c | 1251 +++++ > drivers/net/ethernet/intel/idpf/idpf_txrx.c | 4855 +++++++++++++++++ > drivers/net/ethernet/intel/idpf/idpf_txrx.h | 854 +++ > drivers/net/ethernet/intel/idpf/idpf_vf_dev.c | 164 + > .../net/ethernet/intel/idpf/idpf_virtchnl.c | 3820 +++++++++++++ > drivers/net/ethernet/intel/idpf/virtchnl2.h | 1301 +++++ > .../ethernet/intel/idpf/virtchnl2_lan_desc.h | 448 ++ > 25 files changed, 19437 insertions(+) > create mode 100644 Documentation/networking/device_drivers/ethernet/intel/idpf.rst > create mode 100644 drivers/net/ethernet/intel/idpf/Makefile > create mode 100644 drivers/net/ethernet/intel/idpf/idpf.h > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_controlq.c > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_controlq.h > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_controlq_api.h > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_controlq_setup.c > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_dev.c > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_devids.h > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_ethtool.c > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_lan_pf_regs.h > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_lan_txrx.h > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_lan_vf_regs.h > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_lib.c > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_main.c > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_mem.h > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_singleq_txrx.c > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_txrx.c > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_txrx.h > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_vf_dev.c > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_virtchnl.c > create mode 100644 drivers/net/ethernet/intel/idpf/virtchnl2.h > create mode 100644 drivers/net/ethernet/intel/idpf/virtchnl2_lan_desc.h > > -- > 2.17.2 >
On 5/11/2023 11:34 PM, Michael S. Tsirkin wrote: > On Mon, May 08, 2023 at 12:43:11PM -0700, Emil Tantilov wrote: >> This patch series introduces the Intel Infrastructure Data Path Function >> (IDPF) driver. It is used for both physical and virtual functions. Except >> for some of the device operations the rest of the functionality is the >> same for both PF and VF. IDPF uses virtchnl version2 opcodes and >> structures defined in the virtchnl2 header file which helps the driver >> to learn the capabilities and register offsets from the device >> Control Plane (CP) instead of assuming the default values. > > So, is this for merge in the next cycle? Should this be an RFC rather? > It seems unlikely that the IDPF specification will be finalized by that > time - how are you going to handle any specification changes? Yes. we would like this driver to be merged in the next cycle(6.5). Based on the community feedback on v1 version of the driver, we removed all references to OASIS standard and at this time this is an intel vendor driver. Links to v1 and v2 discussion threads https://lore.kernel.org/netdev/20230329140404.1647925-1-pavan.kumar.linga@intel.com/ https://lore.kernel.org/netdev/20230411011354.2619359-1-pavan.kumar.linga@intel.com/ The v1->v2 change log reflects this update. v1 --> v2: link [1] * removed the OASIS reference in the commit message to make it clear that this is an Intel vendor specific driver Any IDPF specification updates would be handled as part of the changes that would be required to make this a common standards driver.
On Thu, May 18, 2023 at 09:19:31AM -0700, Samudrala, Sridhar wrote: > > > On 5/11/2023 11:34 PM, Michael S. Tsirkin wrote: > > On Mon, May 08, 2023 at 12:43:11PM -0700, Emil Tantilov wrote: > > > This patch series introduces the Intel Infrastructure Data Path Function > > > (IDPF) driver. It is used for both physical and virtual functions. Except > > > for some of the device operations the rest of the functionality is the > > > same for both PF and VF. IDPF uses virtchnl version2 opcodes and > > > structures defined in the virtchnl2 header file which helps the driver > > > to learn the capabilities and register offsets from the device > > > Control Plane (CP) instead of assuming the default values. > > > > So, is this for merge in the next cycle? Should this be an RFC rather? > > It seems unlikely that the IDPF specification will be finalized by that > > time - how are you going to handle any specification changes? > > Yes. we would like this driver to be merged in the next cycle(6.5). > Based on the community feedback on v1 version of the driver, we removed all > references to OASIS standard and at this time this is an intel vendor > driver. > > Links to v1 and v2 discussion threads > https://lore.kernel.org/netdev/20230329140404.1647925-1-pavan.kumar.linga@intel.com/ > https://lore.kernel.org/netdev/20230411011354.2619359-1-pavan.kumar.linga@intel.com/ > > The v1->v2 change log reflects this update. > v1 --> v2: link [1] > * removed the OASIS reference in the commit message to make it clear > that this is an Intel vendor specific driver Yes this makes sense. > Any IDPF specification updates would be handled as part of the changes that > would be required to make this a common standards driver. So my question is, would it make sense to update Kconfig and module name to be "ipu" or if you prefer "intel-idpf" to make it clear this is currently an Intel vendor specific driver? And then when you make it a common standards driver rename it to idpf? The point being to help make sure users are not confused about whether they got a driver with or without IDPF updates. It's not critical I guess but seems like a good idea. WDYT?
On 5/18/2023 10:10 AM, Michael S. Tsirkin wrote: > On Thu, May 18, 2023 at 09:19:31AM -0700, Samudrala, Sridhar wrote: >> >> >> On 5/11/2023 11:34 PM, Michael S. Tsirkin wrote: >>> On Mon, May 08, 2023 at 12:43:11PM -0700, Emil Tantilov wrote: >>>> This patch series introduces the Intel Infrastructure Data Path Function >>>> (IDPF) driver. It is used for both physical and virtual functions. Except >>>> for some of the device operations the rest of the functionality is the >>>> same for both PF and VF. IDPF uses virtchnl version2 opcodes and >>>> structures defined in the virtchnl2 header file which helps the driver >>>> to learn the capabilities and register offsets from the device >>>> Control Plane (CP) instead of assuming the default values. >>> >>> So, is this for merge in the next cycle? Should this be an RFC rather? >>> It seems unlikely that the IDPF specification will be finalized by that >>> time - how are you going to handle any specification changes? >> >> Yes. we would like this driver to be merged in the next cycle(6.5). >> Based on the community feedback on v1 version of the driver, we removed all >> references to OASIS standard and at this time this is an intel vendor >> driver. >> >> Links to v1 and v2 discussion threads >> https://lore.kernel.org/netdev/20230329140404.1647925-1-pavan.kumar.linga@intel.com/ >> https://lore.kernel.org/netdev/20230411011354.2619359-1-pavan.kumar.linga@intel.com/ >> >> The v1->v2 change log reflects this update. >> v1 --> v2: link [1] >> * removed the OASIS reference in the commit message to make it clear >> that this is an Intel vendor specific driver > > Yes this makes sense. > > >> Any IDPF specification updates would be handled as part of the changes that >> would be required to make this a common standards driver. > > > So my question is, would it make sense to update Kconfig and module name > to be "ipu" or if you prefer "intel-idpf" to make it clear this is > currently an Intel vendor specific driver? And then when you make it a > common standards driver rename it to idpf? The point being to help make > sure users are not confused about whether they got a driver with > or without IDPF updates. It's not critical I guess but seems like a good > idea. WDYT? It would be more disruptive to change the name of the driver. We can update the pci device table, module description and possibly driver version when we are ready to make this a standard driver. So we would prefer not changing the driver name.
On Thu, May 18, 2023 at 04:26:24PM -0700, Samudrala, Sridhar wrote: > > > On 5/18/2023 10:10 AM, Michael S. Tsirkin wrote: > > On Thu, May 18, 2023 at 09:19:31AM -0700, Samudrala, Sridhar wrote: > > > > > > > > > On 5/11/2023 11:34 PM, Michael S. Tsirkin wrote: > > > > On Mon, May 08, 2023 at 12:43:11PM -0700, Emil Tantilov wrote: > > > > > This patch series introduces the Intel Infrastructure Data Path Function > > > > > (IDPF) driver. It is used for both physical and virtual functions. Except > > > > > for some of the device operations the rest of the functionality is the > > > > > same for both PF and VF. IDPF uses virtchnl version2 opcodes and > > > > > structures defined in the virtchnl2 header file which helps the driver > > > > > to learn the capabilities and register offsets from the device > > > > > Control Plane (CP) instead of assuming the default values. > > > > > > > > So, is this for merge in the next cycle? Should this be an RFC rather? > > > > It seems unlikely that the IDPF specification will be finalized by that > > > > time - how are you going to handle any specification changes? > > > > > > Yes. we would like this driver to be merged in the next cycle(6.5). > > > Based on the community feedback on v1 version of the driver, we removed all > > > references to OASIS standard and at this time this is an intel vendor > > > driver. > > > > > > Links to v1 and v2 discussion threads > > > https://lore.kernel.org/netdev/20230329140404.1647925-1-pavan.kumar.linga@intel.com/ > > > https://lore.kernel.org/netdev/20230411011354.2619359-1-pavan.kumar.linga@intel.com/ > > > > > > The v1->v2 change log reflects this update. > > > v1 --> v2: link [1] > > > * removed the OASIS reference in the commit message to make it clear > > > that this is an Intel vendor specific driver > > > > Yes this makes sense. > > > > > > > Any IDPF specification updates would be handled as part of the changes that > > > would be required to make this a common standards driver. > > > > > > So my question is, would it make sense to update Kconfig and module name > > to be "ipu" or if you prefer "intel-idpf" to make it clear this is > > currently an Intel vendor specific driver? And then when you make it a > > common standards driver rename it to idpf? The point being to help make > > sure users are not confused about whether they got a driver with > > or without IDPF updates. It's not critical I guess but seems like a good > > idea. WDYT? > > It would be more disruptive to change the name of the driver. We can update > the pci device table, module description and possibly driver version when we > are ready to make this a standard driver. > So we would prefer not changing the driver name. Kconfig entry and description too?
On 5/18/23 4:26 PM, Samudrala, Sridhar wrote: > On 5/18/2023 10:10 AM, Michael S. Tsirkin wrote: >> On Thu, May 18, 2023 at 09:19:31AM -0700, Samudrala, Sridhar wrote: >>> >>> >>> On 5/11/2023 11:34 PM, Michael S. Tsirkin wrote: >>>> On Mon, May 08, 2023 at 12:43:11PM -0700, Emil Tantilov wrote: >>>>> This patch series introduces the Intel Infrastructure Data Path >>>>> Function >>>>> (IDPF) driver. It is used for both physical and virtual functions. >>>>> Except >>>>> for some of the device operations the rest of the functionality is the >>>>> same for both PF and VF. IDPF uses virtchnl version2 opcodes and >>>>> structures defined in the virtchnl2 header file which helps the driver >>>>> to learn the capabilities and register offsets from the device >>>>> Control Plane (CP) instead of assuming the default values. >>>> >>>> So, is this for merge in the next cycle? Should this be an RFC rather? >>>> It seems unlikely that the IDPF specification will be finalized by that >>>> time - how are you going to handle any specification changes? >>> >>> Yes. we would like this driver to be merged in the next cycle(6.5). >>> Based on the community feedback on v1 version of the driver, we >>> removed all >>> references to OASIS standard and at this time this is an intel vendor >>> driver. >>> >>> Links to v1 and v2 discussion threads >>> https://lore.kernel.org/netdev/20230329140404.1647925-1-pavan.kumar.linga@intel.com/ >>> https://lore.kernel.org/netdev/20230411011354.2619359-1-pavan.kumar.linga@intel.com/ >>> >>> The v1->v2 change log reflects this update. >>> v1 --> v2: link [1] >>> * removed the OASIS reference in the commit message to make it clear >>> that this is an Intel vendor specific driver >> >> Yes this makes sense. >> >> >>> Any IDPF specification updates would be handled as part of the >>> changes that >>> would be required to make this a common standards driver. >> >> >> So my question is, would it make sense to update Kconfig and module name >> to be "ipu" or if you prefer "intel-idpf" to make it clear this is >> currently an Intel vendor specific driver? And then when you make it a >> common standards driver rename it to idpf? The point being to help make >> sure users are not confused about whether they got a driver with >> or without IDPF updates. It's not critical I guess but seems like a good >> idea. WDYT? > > It would be more disruptive to change the name of the driver. We can > update the pci device table, module description and possibly driver > version when we are ready to make this a standard driver. > So we would prefer not changing the driver name. More disruptive for who? I think it would be better to change the name of the one driver now before a problem is created in the tree than to leave a point of confusion for the rest of the drivers to contend with in the future. sln
On Fri, May 19, 2023 at 12:17 PM Shannon Nelson <shannon.nelson@amd.com> wrote: > > On 5/18/23 4:26 PM, Samudrala, Sridhar wrote: > > On 5/18/2023 10:10 AM, Michael S. Tsirkin wrote: > >> On Thu, May 18, 2023 at 09:19:31AM -0700, Samudrala, Sridhar wrote: > >>> > >>> > >>> On 5/11/2023 11:34 PM, Michael S. Tsirkin wrote: > >>>> On Mon, May 08, 2023 at 12:43:11PM -0700, Emil Tantilov wrote: > >>>>> This patch series introduces the Intel Infrastructure Data Path > >>>>> Function > >>>>> (IDPF) driver. It is used for both physical and virtual functions. > >>>>> Except > >>>>> for some of the device operations the rest of the functionality is the > >>>>> same for both PF and VF. IDPF uses virtchnl version2 opcodes and > >>>>> structures defined in the virtchnl2 header file which helps the driver > >>>>> to learn the capabilities and register offsets from the device > >>>>> Control Plane (CP) instead of assuming the default values. > >>>> > >>>> So, is this for merge in the next cycle? Should this be an RFC rather? > >>>> It seems unlikely that the IDPF specification will be finalized by that > >>>> time - how are you going to handle any specification changes? > >>> > >>> Yes. we would like this driver to be merged in the next cycle(6.5). > >>> Based on the community feedback on v1 version of the driver, we > >>> removed all > >>> references to OASIS standard and at this time this is an intel vendor > >>> driver. > >>> > >>> Links to v1 and v2 discussion threads > >>> https://lore.kernel.org/netdev/20230329140404.1647925-1-pavan.kumar.linga@intel.com/ > >>> https://lore.kernel.org/netdev/20230411011354.2619359-1-pavan.kumar.linga@intel.com/ > >>> > >>> The v1->v2 change log reflects this update. > >>> v1 --> v2: link [1] > >>> * removed the OASIS reference in the commit message to make it clear > >>> that this is an Intel vendor specific driver > >> > >> Yes this makes sense. > >> > >> > >>> Any IDPF specification updates would be handled as part of the > >>> changes that > >>> would be required to make this a common standards driver. > >> > >> > >> So my question is, would it make sense to update Kconfig and module name > >> to be "ipu" or if you prefer "intel-idpf" to make it clear this is > >> currently an Intel vendor specific driver? And then when you make it a > >> common standards driver rename it to idpf? The point being to help make > >> sure users are not confused about whether they got a driver with > >> or without IDPF updates. It's not critical I guess but seems like a good > >> idea. WDYT? > > > > It would be more disruptive to change the name of the driver. We can > > update the pci device table, module description and possibly driver > > version when we are ready to make this a standard driver. > > So we would prefer not changing the driver name. > > More disruptive for who? > > I think it would be better to change the name of the one driver now > before a problem is created in the tree than to leave a point of > confusion for the rest of the drivers to contend with in the future. This discussion is premised on the idea that the drivers will inevitably fork, with an Intel driver and a non-backward compatible standardized driver. Instead, I expect that the goal is that the future standardized driver will iterate and support additional features. But that the existing hardware will continue to be supported, if perhaps with updated firmware. IDPF from the start uses feature negotiation over virtchannel to be highly configurable. A future driver might deprecate older feature (variants), while either still continue to support those or require firmware updates to match the new version. Even if the device API would break in a non-backward compatible way, the same driver can support both versions. Virtio is an example of this. If I'm wrong and for some reason two drivers would have to be supported, then I'm sure we can figure out a suffix or prefix to the standard driver that separates it from the existing one.
On 5/18/2023 10:49 PM, Michael S. Tsirkin wrote: > On Thu, May 18, 2023 at 04:26:24PM -0700, Samudrala, Sridhar wrote: >> >> >> On 5/18/2023 10:10 AM, Michael S. Tsirkin wrote: >>> On Thu, May 18, 2023 at 09:19:31AM -0700, Samudrala, Sridhar wrote: >>>> >>>> >>>> On 5/11/2023 11:34 PM, Michael S. Tsirkin wrote: >>>>> On Mon, May 08, 2023 at 12:43:11PM -0700, Emil Tantilov wrote: >>>>>> This patch series introduces the Intel Infrastructure Data Path Function >>>>>> (IDPF) driver. It is used for both physical and virtual functions. Except >>>>>> for some of the device operations the rest of the functionality is the >>>>>> same for both PF and VF. IDPF uses virtchnl version2 opcodes and >>>>>> structures defined in the virtchnl2 header file which helps the driver >>>>>> to learn the capabilities and register offsets from the device >>>>>> Control Plane (CP) instead of assuming the default values. >>>>> >>>>> So, is this for merge in the next cycle? Should this be an RFC rather? >>>>> It seems unlikely that the IDPF specification will be finalized by that >>>>> time - how are you going to handle any specification changes? >>>> >>>> Yes. we would like this driver to be merged in the next cycle(6.5). >>>> Based on the community feedback on v1 version of the driver, we removed all >>>> references to OASIS standard and at this time this is an intel vendor >>>> driver. >>>> >>>> Links to v1 and v2 discussion threads >>>> https://lore.kernel.org/netdev/20230329140404.1647925-1-pavan.kumar.linga@intel.com/ >>>> https://lore.kernel.org/netdev/20230411011354.2619359-1-pavan.kumar.linga@intel.com/ >>>> >>>> The v1->v2 change log reflects this update. >>>> v1 --> v2: link [1] >>>> * removed the OASIS reference in the commit message to make it clear >>>> that this is an Intel vendor specific driver >>> >>> Yes this makes sense. >>> >>> >>>> Any IDPF specification updates would be handled as part of the changes that >>>> would be required to make this a common standards driver. >>> >>> >>> So my question is, would it make sense to update Kconfig and module name >>> to be "ipu" or if you prefer "intel-idpf" to make it clear this is >>> currently an Intel vendor specific driver? And then when you make it a >>> common standards driver rename it to idpf? The point being to help make >>> sure users are not confused about whether they got a driver with >>> or without IDPF updates. It's not critical I guess but seems like a good >>> idea. WDYT? >> >> It would be more disruptive to change the name of the driver. We can update >> the pci device table, module description and possibly driver version when we >> are ready to make this a standard driver. >> So we would prefer not changing the driver name. > > Kconfig entry and description too? > The current Kconfig entry has Intel references. +config IDPF + tristate "Intel(R) Infrastructure Data Path Function Support" + depends on PCI_MSI + select DIMLIB + help + This driver supports Intel(R) Infrastructure Processing Unit (IPU) + devices. It can be updated with Intel references removed when the spec becomes standard and meets the community requirements.
> +config IDPF > + tristate "Intel(R) Infrastructure Data Path Function Support" > + depends on PCI_MSI > + select DIMLIB > + help > + This driver supports Intel(R) Infrastructure Processing Unit (IPU) > + devices. > > It can be updated with Intel references removed when the spec becomes > standard and meets the community requirements. Is IPU Intels name for the hardware which implements DPF? I assume when 'Intel' is dropped, IPU would also be dropped? Which leaves the help empty. And i assume when it is no longer tied to Intel, the Kconfig entry will move somewhere else, because at the moment, it appears to appear under Intel, when it probably should be at a higher level, maybe 'Network device support'? And will the code maybe move to net/idpf? <tongue in cheek> Maybe put it into driver/staging/idpf for the moment? </tongue in cheek> Andrew
On Fri, May 19, 2023 at 2:22 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > +config IDPF > > + tristate "Intel(R) Infrastructure Data Path Function Support" > > + depends on PCI_MSI > > + select DIMLIB > > + help > > + This driver supports Intel(R) Infrastructure Processing Unit (IPU) > > + devices. > > > > It can be updated with Intel references removed when the spec becomes > > standard and meets the community requirements. > > Is IPU Intels name for the hardware which implements DPF? I assume > when 'Intel' is dropped, IPU would also be dropped? Which leaves the > help empty. > > And i assume when it is no longer tied to Intel, the Kconfig entry > will move somewhere else, because at the moment, it appears to appear > under Intel, when it probably should be at a higher level, maybe > 'Network device support'? And will the code maybe move to net/idpf? This has come up before. "Drivers are organized by the vendor for better or worse. We have a number of drivers under the "wrong directly" already. Companies merge / buy each others product lines, there's also some confusion about common IP drivers. It's all fine, whatever." https://lore.kernel.org/netdev/20230414152744.4fd219f9@kernel.org/
On Fri, May 19, 2023 at 10:36:00AM -0700, Samudrala, Sridhar wrote: > > > On 5/18/2023 10:49 PM, Michael S. Tsirkin wrote: > > On Thu, May 18, 2023 at 04:26:24PM -0700, Samudrala, Sridhar wrote: > > > > > > > > > On 5/18/2023 10:10 AM, Michael S. Tsirkin wrote: > > > > On Thu, May 18, 2023 at 09:19:31AM -0700, Samudrala, Sridhar wrote: > > > > > > > > > > > > > > > On 5/11/2023 11:34 PM, Michael S. Tsirkin wrote: > > > > > > On Mon, May 08, 2023 at 12:43:11PM -0700, Emil Tantilov wrote: > > > > > > > This patch series introduces the Intel Infrastructure Data Path Function > > > > > > > (IDPF) driver. It is used for both physical and virtual functions. Except > > > > > > > for some of the device operations the rest of the functionality is the > > > > > > > same for both PF and VF. IDPF uses virtchnl version2 opcodes and > > > > > > > structures defined in the virtchnl2 header file which helps the driver > > > > > > > to learn the capabilities and register offsets from the device > > > > > > > Control Plane (CP) instead of assuming the default values. > > > > > > > > > > > > So, is this for merge in the next cycle? Should this be an RFC rather? > > > > > > It seems unlikely that the IDPF specification will be finalized by that > > > > > > time - how are you going to handle any specification changes? > > > > > > > > > > Yes. we would like this driver to be merged in the next cycle(6.5). > > > > > Based on the community feedback on v1 version of the driver, we removed all > > > > > references to OASIS standard and at this time this is an intel vendor > > > > > driver. > > > > > > > > > > Links to v1 and v2 discussion threads > > > > > https://lore.kernel.org/netdev/20230329140404.1647925-1-pavan.kumar.linga@intel.com/ > > > > > https://lore.kernel.org/netdev/20230411011354.2619359-1-pavan.kumar.linga@intel.com/ > > > > > > > > > > The v1->v2 change log reflects this update. > > > > > v1 --> v2: link [1] > > > > > * removed the OASIS reference in the commit message to make it clear > > > > > that this is an Intel vendor specific driver > > > > > > > > Yes this makes sense. > > > > > > > > > > > > > Any IDPF specification updates would be handled as part of the changes that > > > > > would be required to make this a common standards driver. > > > > > > > > > > > > So my question is, would it make sense to update Kconfig and module name > > > > to be "ipu" or if you prefer "intel-idpf" to make it clear this is > > > > currently an Intel vendor specific driver? And then when you make it a > > > > common standards driver rename it to idpf? The point being to help make > > > > sure users are not confused about whether they got a driver with > > > > or without IDPF updates. It's not critical I guess but seems like a good > > > > idea. WDYT? > > > > > > It would be more disruptive to change the name of the driver. We can update > > > the pci device table, module description and possibly driver version when we > > > are ready to make this a standard driver. > > > So we would prefer not changing the driver name. > > > > Kconfig entry and description too? > > > > The current Kconfig entry has Intel references. > > +config IDPF > + tristate "Intel(R) Infrastructure Data Path Function Support" > + depends on PCI_MSI > + select DIMLIB > + help > + This driver supports Intel(R) Infrastructure Processing Unit (IPU) > + devices. > > It can be updated with Intel references removed when the spec becomes > standard and meets the community requirements. Right, name says IDPF support help says IPU support. Also config does not match name. Do you want: config INTEL_IDPF tristate "Intel(R) Infrastructure Data Path Function Support" and should help say This driver supports Intel(R) Infrastructure Data Path Function devices. ?
On 5/21/2023 2:21 AM, Michael S. Tsirkin wrote: > On Fri, May 19, 2023 at 10:36:00AM -0700, Samudrala, Sridhar wrote: >> >> >> On 5/18/2023 10:49 PM, Michael S. Tsirkin wrote: >>> On Thu, May 18, 2023 at 04:26:24PM -0700, Samudrala, Sridhar wrote: >>>> >>>> >>>> On 5/18/2023 10:10 AM, Michael S. Tsirkin wrote: >>>>> On Thu, May 18, 2023 at 09:19:31AM -0700, Samudrala, Sridhar wrote: >>>>>> >>>>>> >>>>>> On 5/11/2023 11:34 PM, Michael S. Tsirkin wrote: >>>>>>> On Mon, May 08, 2023 at 12:43:11PM -0700, Emil Tantilov wrote: >>>>>>>> This patch series introduces the Intel Infrastructure Data Path Function >>>>>>>> (IDPF) driver. It is used for both physical and virtual functions. Except >>>>>>>> for some of the device operations the rest of the functionality is the >>>>>>>> same for both PF and VF. IDPF uses virtchnl version2 opcodes and >>>>>>>> structures defined in the virtchnl2 header file which helps the driver >>>>>>>> to learn the capabilities and register offsets from the device >>>>>>>> Control Plane (CP) instead of assuming the default values. >>>>>>> >>>>>>> So, is this for merge in the next cycle? Should this be an RFC rather? >>>>>>> It seems unlikely that the IDPF specification will be finalized by that >>>>>>> time - how are you going to handle any specification changes? >>>>>> >>>>>> Yes. we would like this driver to be merged in the next cycle(6.5). >>>>>> Based on the community feedback on v1 version of the driver, we removed all >>>>>> references to OASIS standard and at this time this is an intel vendor >>>>>> driver. >>>>>> >>>>>> Links to v1 and v2 discussion threads >>>>>> https://lore.kernel.org/netdev/20230329140404.1647925-1-pavan.kumar.linga@intel.com/ >>>>>> https://lore.kernel.org/netdev/20230411011354.2619359-1-pavan.kumar.linga@intel.com/ >>>>>> >>>>>> The v1->v2 change log reflects this update. >>>>>> v1 --> v2: link [1] >>>>>> * removed the OASIS reference in the commit message to make it clear >>>>>> that this is an Intel vendor specific driver >>>>> >>>>> Yes this makes sense. >>>>> >>>>> >>>>>> Any IDPF specification updates would be handled as part of the changes that >>>>>> would be required to make this a common standards driver. >>>>> >>>>> >>>>> So my question is, would it make sense to update Kconfig and module name >>>>> to be "ipu" or if you prefer "intel-idpf" to make it clear this is >>>>> currently an Intel vendor specific driver? And then when you make it a >>>>> common standards driver rename it to idpf? The point being to help make >>>>> sure users are not confused about whether they got a driver with >>>>> or without IDPF updates. It's not critical I guess but seems like a good >>>>> idea. WDYT? >>>> >>>> It would be more disruptive to change the name of the driver. We can update >>>> the pci device table, module description and possibly driver version when we >>>> are ready to make this a standard driver. >>>> So we would prefer not changing the driver name. >>> >>> Kconfig entry and description too? >>> >> >> The current Kconfig entry has Intel references. >> >> +config IDPF >> + tristate "Intel(R) Infrastructure Data Path Function Support" >> + depends on PCI_MSI >> + select DIMLIB >> + help >> + This driver supports Intel(R) Infrastructure Processing Unit (IPU) >> + devices. >> >> It can be updated with Intel references removed when the spec becomes >> standard and meets the community requirements. > > Right, name says IDPF support help says IPU support. > Also config does not match name. > > Do you want: > > > config INTEL_IDPF > tristate "Intel(R) Infrastructure Data Path Function Support" > > and should help say > > This driver supports Intel(R) Infrastructure Data Path Function > devices. > ? IDPF Kconfig entry is listed only when CONFIG_NET_VENDOR_INTEL is selected. So I think adding INTEL_ prefix to the config entry under Intel devices sounds redundant. But we can definitely update the help section as you suggested to match with the name.
On Fri, May 19, 2023 at 01:12:43PM -0400, Willem de Bruijn wrote: > On Fri, May 19, 2023 at 12:17 PM Shannon Nelson <shannon.nelson@amd.com> wrote: > > > > On 5/18/23 4:26 PM, Samudrala, Sridhar wrote: > > > On 5/18/2023 10:10 AM, Michael S. Tsirkin wrote: > > >> On Thu, May 18, 2023 at 09:19:31AM -0700, Samudrala, Sridhar wrote: > > >>> > > >>> > > >>> On 5/11/2023 11:34 PM, Michael S. Tsirkin wrote: > > >>>> On Mon, May 08, 2023 at 12:43:11PM -0700, Emil Tantilov wrote: > > >>>>> This patch series introduces the Intel Infrastructure Data Path > > >>>>> Function > > >>>>> (IDPF) driver. It is used for both physical and virtual functions. > > >>>>> Except > > >>>>> for some of the device operations the rest of the functionality is the > > >>>>> same for both PF and VF. IDPF uses virtchnl version2 opcodes and > > >>>>> structures defined in the virtchnl2 header file which helps the driver > > >>>>> to learn the capabilities and register offsets from the device > > >>>>> Control Plane (CP) instead of assuming the default values. > > >>>> > > >>>> So, is this for merge in the next cycle? Should this be an RFC rather? > > >>>> It seems unlikely that the IDPF specification will be finalized by that > > >>>> time - how are you going to handle any specification changes? > > >>> > > >>> Yes. we would like this driver to be merged in the next cycle(6.5). > > >>> Based on the community feedback on v1 version of the driver, we > > >>> removed all > > >>> references to OASIS standard and at this time this is an intel vendor > > >>> driver. > > >>> > > >>> Links to v1 and v2 discussion threads > > >>> https://lore.kernel.org/netdev/20230329140404.1647925-1-pavan.kumar.linga@intel.com/ > > >>> https://lore.kernel.org/netdev/20230411011354.2619359-1-pavan.kumar.linga@intel.com/ > > >>> > > >>> The v1->v2 change log reflects this update. > > >>> v1 --> v2: link [1] > > >>> * removed the OASIS reference in the commit message to make it clear > > >>> that this is an Intel vendor specific driver > > >> > > >> Yes this makes sense. > > >> > > >> > > >>> Any IDPF specification updates would be handled as part of the > > >>> changes that > > >>> would be required to make this a common standards driver. > > >> > > >> > > >> So my question is, would it make sense to update Kconfig and module name > > >> to be "ipu" or if you prefer "intel-idpf" to make it clear this is > > >> currently an Intel vendor specific driver? And then when you make it a > > >> common standards driver rename it to idpf? The point being to help make > > >> sure users are not confused about whether they got a driver with > > >> or without IDPF updates. It's not critical I guess but seems like a good > > >> idea. WDYT? > > > > > > It would be more disruptive to change the name of the driver. We can > > > update the pci device table, module description and possibly driver > > > version when we are ready to make this a standard driver. > > > So we would prefer not changing the driver name. > > > > More disruptive for who? > > > > I think it would be better to change the name of the one driver now > > before a problem is created in the tree than to leave a point of > > confusion for the rest of the drivers to contend with in the future. > > This discussion is premised on the idea that the drivers will > inevitably fork, with an Intel driver and a non-backward compatible > standardized driver. > > Instead, I expect that the goal is that the future standardized driver > will iterate and support additional features. But that the existing > hardware will continue to be supported, if perhaps with updated > firmware. > > IDPF from the start uses feature negotiation over virtchannel to be > highly configurable. A future driver might deprecate older feature > (variants), while either still continue to support those or require > firmware updates to match the new version. > > Even if the device API would break in a non-backward compatible way, > the same driver can support both versions. Virtio is an example of > this. > > If I'm wrong and for some reason two drivers would have to be > supported, then I'm sure we can figure out a suffix or prefix to the > standard driver that separates it from the existing one. OK let us look at a more specific example. The IDPF TC recently voted to bind to device based on class/programming interface as opposed to device/vendor id. Future driver will likely do this. Current driver only binds to intel's device and vendor id. Assuming this happens, what bothers me is that depending on kernel version, driver idpf.ko either does or does not bind to idpf programming interface. All this is quite imminent. Yes tricks like checking module version to check what is supported is possible, but we are beginning to already develop technical dept and lore and we just started. Simple lsmod|grep -q idpf seems like an easier, more robust and intuitive way than if [[ $(cat /sys/module/idpf/version) == "2.0" ]] then echo "idpf" fi And yes somemore might ship the existing driver during this initial window. Then we get lsmod|grep -q -e idpf -e intel-ipu still seems pretty clean. And hopefully the change will happen within a couple of months so not more than one release will have the intel-ipu name. Of course all this is not earth shatteringly important but still, I'm interested in what others think.
I agree on Help message change as it is not accurate now and I like MST's suggestion. Anjali -----Original Message----- From: Michael S. Tsirkin <mst@redhat.com> Sent: Sunday, May 21, 2023 2:21 AM To: Samudrala, Sridhar <sridhar.samudrala@intel.com> Cc: Tantilov, Emil S <emil.s.tantilov@intel.com>; intel-wired-lan@lists.osuosl.org; shannon.nelson@amd.com; simon.horman@corigine.com; leon@kernel.org; decot@google.com; willemb@google.com; Brandeburg, Jesse <jesse.brandeburg@intel.com>; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; davem@davemloft.net; edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; netdev@vger.kernel.org; Singhai, Anjali <anjali.singhai@intel.com>; Orr, Michael <michael.orr@intel.com> Subject: Re: [PATCH iwl-next v4 00/15] Introduce Intel IDPF driver On Fri, May 19, 2023 at 10:36:00AM -0700, Samudrala, Sridhar wrote: > > > On 5/18/2023 10:49 PM, Michael S. Tsirkin wrote: > > On Thu, May 18, 2023 at 04:26:24PM -0700, Samudrala, Sridhar wrote: > > > > > > > > > On 5/18/2023 10:10 AM, Michael S. Tsirkin wrote: > > > > On Thu, May 18, 2023 at 09:19:31AM -0700, Samudrala, Sridhar wrote: > > > > > > > > > > > > > > > On 5/11/2023 11:34 PM, Michael S. Tsirkin wrote: > > > > > > On Mon, May 08, 2023 at 12:43:11PM -0700, Emil Tantilov wrote: > > > > > > > This patch series introduces the Intel Infrastructure Data > > > > > > > Path Function > > > > > > > (IDPF) driver. It is used for both physical and virtual > > > > > > > functions. Except for some of the device operations the > > > > > > > rest of the functionality is the same for both PF and VF. > > > > > > > IDPF uses virtchnl version2 opcodes and structures defined > > > > > > > in the virtchnl2 header file which helps the driver to > > > > > > > learn the capabilities and register offsets from the device Control Plane (CP) instead of assuming the default values. > > > > > > > > > > > > So, is this for merge in the next cycle? Should this be an RFC rather? > > > > > > It seems unlikely that the IDPF specification will be > > > > > > finalized by that time - how are you going to handle any specification changes? > > > > > > > > > > Yes. we would like this driver to be merged in the next cycle(6.5). > > > > > Based on the community feedback on v1 version of the driver, > > > > > we removed all references to OASIS standard and at this time > > > > > this is an intel vendor driver. > > > > > > > > > > Links to v1 and v2 discussion threads > > > > > https://lore.kernel.org/netdev/20230329140404.1647925-1-pavan. > > > > > kumar.linga@intel.com/ > > > > > https://lore.kernel.org/netdev/20230411011354.2619359-1-pavan. > > > > > kumar.linga@intel.com/ > > > > > > > > > > The v1->v2 change log reflects this update. > > > > > v1 --> v2: link [1] > > > > > * removed the OASIS reference in the commit message to make it clear > > > > > that this is an Intel vendor specific driver > > > > > > > > Yes this makes sense. > > > > > > > > > > > > > Any IDPF specification updates would be handled as part of the > > > > > changes that would be required to make this a common standards driver. > > > > > > > > > > > > So my question is, would it make sense to update Kconfig and > > > > module name to be "ipu" or if you prefer "intel-idpf" to make it > > > > clear this is currently an Intel vendor specific driver? And > > > > then when you make it a common standards driver rename it to > > > > idpf? The point being to help make sure users are not confused > > > > about whether they got a driver with or without IDPF updates. > > > > It's not critical I guess but seems like a good idea. WDYT? > > > > > > It would be more disruptive to change the name of the driver. We > > > can update the pci device table, module description and possibly > > > driver version when we are ready to make this a standard driver. > > > So we would prefer not changing the driver name. > > > > Kconfig entry and description too? > > > > The current Kconfig entry has Intel references. > > +config IDPF > + tristate "Intel(R) Infrastructure Data Path Function Support" > + depends on PCI_MSI > + select DIMLIB > + help > + This driver supports Intel(R) Infrastructure Processing Unit (IPU) > + devices. > > It can be updated with Intel references removed when the spec becomes > standard and meets the community requirements. Right, name says IDPF support help says IPU support. Also config does not match name. Do you want: config INTEL_IDPF tristate "Intel(R) Infrastructure Data Path Function Support" and should help say This driver supports Intel(R) Infrastructure Data Path Function devices. ?
On Mon, May 22, 2023 at 08:08:47PM +0000, Singhai, Anjali wrote: > I agree on Help message change as it is not accurate now and I like MST's suggestion. > Hi Anjali Please don't top post. Also, please trim your replies. All the standard network etiquette things. Andrew