[01/10] ethdev: introduce MACSEC device ops

  MACSEC related device ops, API and parameters are taken from the
existing ixgbe PMD ops

Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
 lib/librte_ethdev/rte_ethdev.c      |  87 +++++++++++++++++++++
 lib/librte_ethdev/rte_ethdev.h      | 115 ++++++++++++++++++++++++++++
 lib/librte_ethdev/rte_ethdev_core.h |  23 ++++++
 3 files changed, 225 insertions(+)
  

10/04/2019 13:18, Igor Russkikh:
> MACSEC related device ops, API and parameters are taken from the
> existing ixgbe PMD ops

Please can you explain how it is related to rte_security?
  

Hello Thomas,

Thanks for the review!

On 10.04.2019 14:46, Thomas Monjalon wrote:
> 10/04/2019 13:18, Igor Russkikh:
>> MACSEC related device ops, API and parameters are taken from the
>> existing ixgbe PMD ops
> 
> Please can you explain how it is related to rte_security?

It is not.
Do you mean macsec control API could be moved and logically be a part of rte_security api?
I can't comment now on how feasible is this. Moreover this depends on how Intel considers
and uses the existing macsec offload in ixgbe.

> The generic API is implemented in ixgbe in next patch.
> You should reverse them, or completely move this patch at the end.

You are right, I'll swap them.

> Please merge such doc update in the patch enabling the feature.

Ok,

Thanks,
  Igor
  

11/04/2019 14:37, Igor Russkikh:
> On 10.04.2019 14:46, Thomas Monjalon wrote:
> > 10/04/2019 13:18, Igor Russkikh:
> >> MACSEC related device ops, API and parameters are taken from the
> >> existing ixgbe PMD ops
> > 
> > Please can you explain how it is related to rte_security?
> 
> It is not.
> Do you mean macsec control API could be moved and logically be a part of rte_security api?
> I can't comment now on how feasible is this. Moreover this depends on how Intel considers
> and uses the existing macsec offload in ixgbe.

There are RTE_SECURITY_PROTOCOL_MACSEC and rte_security_macsec_* structs
in librte_security.
Please check how it can be used while defining an ethdev API.
  

>>> Please can you explain how it is related to rte_security?
>>
>> It is not.
>> Do you mean macsec control API could be moved and logically be a part of rte_security api?
>> I can't comment now on how feasible is this. Moreover this depends on how Intel considers
>> and uses the existing macsec offload in ixgbe.
> 
> There are RTE_SECURITY_PROTOCOL_MACSEC and rte_security_macsec_* structs
> in librte_security.
> Please check how it can be used while defining an ethdev API.

There is nothing in rte_security defined explicitly for macsec except that enum item.
All the macsec structures are dummy. Was there any intent to implement this?

I can writeup the generic macsec structures for rte_security, do you think that's feasible?

Regards,
  Igor
  

12/04/2019 10:50, Igor Russkikh:
> 
> >>> Please can you explain how it is related to rte_security?
> >>
> >> It is not.
> >> Do you mean macsec control API could be moved and logically be a part of rte_security api?
> >> I can't comment now on how feasible is this. Moreover this depends on how Intel considers
> >> and uses the existing macsec offload in ixgbe.
> > 
> > There are RTE_SECURITY_PROTOCOL_MACSEC and rte_security_macsec_* structs
> > in librte_security.
> > Please check how it can be used while defining an ethdev API.
> 
> There is nothing in rte_security defined explicitly for macsec except that enum item.
> All the macsec structures are dummy. Was there any intent to implement this?
> 
> I can writeup the generic macsec structures for rte_security, do you think that's feasible?

I don't know. We are in front of a case of dead code
written well too far in advance.
Akhil, any suggestion?
  

On 4/10/2019 12:18 PM, Igor Russkikh wrote:
> MACSEC related device ops, API and parameters are taken from the
> existing ixgbe PMD ops
> 
> Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>

<...>

> @@ -3872,6 +3872,121 @@ rte_eth_dev_pool_ops_supported(uint16_t port_id, const char *pool);
>  void *
>  rte_eth_dev_get_sec_ctx(uint16_t port_id);
>  
> +/**
> + * Enable MACsec offload.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param encr
> + *    1 - Enable encryption (encrypt and add integrity signature).
> + *    0 - Disable encryption (only add integrity signature).
> + * @param repl_prot
> + *    1 - Enable replay protection.
> + *    0 - Disable replay protection.
> + * @return
> + *   - (0) if successful.
> + *   - (-ENODEV) if *port* invalid.
> + *   - (-ENOTSUP) if hardware doesn't support this feature.
> + */
> +int
> +rte_eth_macsec_enable(uint16_t port_id,
> +		      uint8_t encr, uint8_t repl_prot);
> +
> +/**
> + * Disable MACsec offload.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @return
> + *   - (0) if successful.
> + *   - (-ENODEV) if *port* invalid.
> + *   - (-ENOTSUP) if hardware doesn't support this feature.
> + */
> +int
> +rte_eth_macsec_disable(uint16_t port_id);
> +
> +/**
> + * Configure Rx SC (Secure Connection).
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param mac
> + *   The MAC address on the remote side.
> + * @param pi
> + *   The PI (port identifier) on the remote side.
> + * @return
> + *   - (0) if successful.
> + *   - (-ENODEV) if *port* invalid.
> + *   - (-ENOTSUP) if hardware doesn't support this feature.
> + */
> +int
> +rte_eth_macsec_config_rxsc(uint16_t port_id,
> +			   uint8_t *mac, uint16_t pi);
> +
> +/**
> + * Configure Tx SC (Secure Connection).
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param mac
> + *   The MAC address on the local side.
> + * @return
> + *   - (0) if successful.
> + *   - (-ENODEV) if *port* invalid.
> + *   - (-ENOTSUP) if hardware doesn't support this feature.
> + */
> +int
> +rte_eth_macsec_config_txsc(uint16_t port_id,
> +			   uint8_t *mac);
> +
> +/**
> + * Enable Rx SA (Secure Association).
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param idx
> + *   The SA to be enabled (0 or 1)
> + * @param an
> + *   The association number on the remote side.
> + * @param pn
> + *   The packet number on the remote side.
> + * @param key
> + *   The key on the remote side.
> + * @return
> + *   - (0) if successful.
> + *   - (-ENODEV) if *port* invalid.
> + *   - (-ENOTSUP) if hardware doesn't support this feature.
> + *   - (-EINVAL) if bad parameter.
> + */
> +int
> +rte_eth_macsec_select_rxsa(uint16_t port_id,
> +			   uint8_t idx, uint8_t an,
> +			   uint32_t pn, uint8_t *key);
> +
> +/**
> + * Enable Tx SA (Secure Association).
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param idx
> + *   The SA to be enabled (0 or 1).
> + * @param an
> + *   The association number on the local side.
> + * @param pn
> + *   The packet number on the local side.
> + * @param key
> + *   The key on the local side.
> + * @return
> + *   - (0) if successful.
> + *   - (-ENODEV) if *port* invalid.
> + *   - (-ENOTSUP) if hardware doesn't support this feature.
> + *   - (-EINVAL) if bad parameter.
> + */
> +int
> +rte_eth_macsec_select_txsa(uint16_t port_id,
> +			   uint8_t idx, uint8_t an,
> +			   uint32_t pn, uint8_t *key);
> +
>  
>  #include <rte_ethdev_core.h>
>  

These are new ethdev APIs, not driver code, that have been sent after rc1, so
these didn't go through a proper review cycle, we didn't get any comment on any
other possible driver can use it, I am for postponing the series to next release.

Also there are some mechanical issues [1] but main thing is adding a set of API
to late in release cycle without proper review.
Thomas, Andrew, what do you think?


[1]
- New APIs must be experimental
- Apis should be exported via linker file (.map)
  

Hi Ferruh,

>> +int
>> +rte_eth_macsec_select_txsa(uint16_t port_id,
>> +			   uint8_t idx, uint8_t an,
>> +			   uint32_t pn, uint8_t *key);
>> +
>>  
>>  #include <rte_ethdev_core.h>
>>  
> 
> These are new ethdev APIs, not driver code, that have been sent after rc1, so
> these didn't go through a proper review cycle, we didn't get any comment on any
> other possible driver can use it, I am for postponing the series to next release.
> 
> Also there are some mechanical issues [1] but main thing is adding a set of API
> to late in release cycle without proper review.

I see, that's reasonable.

May I suggest another option then: can we do driver only API (almost like ixgbe providing now)?
Two points here:

1) Thomas raised a reasonable question whether all the macsec control in general should happen
through the rte_security set of APIs. This obviously could be done, but with proper design
of rte_security structures and ops.

2) Aquantia is interested in having macsec control code in driver within 19.05, even in form
of private driver API as it runs in ixgbe now. This code is functional and will not be
changed alot anyway. It could be easily adopted later when point (1) gets a conclusion.

Thanks for the review,

Regards,
  Igor
  

On 4/13/2019 8:24 AM, Igor Russkikh wrote:
> Hi Ferruh,
> 
>>> +int
>>> +rte_eth_macsec_select_txsa(uint16_t port_id,
>>> +			   uint8_t idx, uint8_t an,
>>> +			   uint32_t pn, uint8_t *key);
>>> +
>>>  
>>>  #include <rte_ethdev_core.h>
>>>  
>>
>> These are new ethdev APIs, not driver code, that have been sent after rc1, so
>> these didn't go through a proper review cycle, we didn't get any comment on any
>> other possible driver can use it, I am for postponing the series to next release.
>>
>> Also there are some mechanical issues [1] but main thing is adding a set of API
>> to late in release cycle without proper review.
> 
> I see, that's reasonable.
> 
> May I suggest another option then: can we do driver only API (almost like ixgbe providing now)?
> Two points here:
> 
> 1) Thomas raised a reasonable question whether all the macsec control in general should happen
> through the rte_security set of APIs. This obviously could be done, but with proper design
> of rte_security structures and ops.
> 
> 2) Aquantia is interested in having macsec control code in driver within 19.05, even in form
> of private driver API as it runs in ixgbe now. This code is functional and will not be
> changed alot anyway. It could be easily adopted later when point (1) gets a conclusion.
> 

If there is a commitment to work on a generic solution for 19.08, involving
other users too, I would be OK to get the support as PMD API for this release.

If that is accepted, please bu sure too add experimental tag to new PMD APIs and
even add to release notes about intention and that the PMD specific APIs are
temporary. And if ABI breakage required, put any necessary deprecation notice
withing this release scope so that the development is not blocked for next release.

Thomas, Andrew, what do you think?
  

On 4/16/19 12:43 PM, Ferruh Yigit wrote:
> On 4/13/2019 8:24 AM, Igor Russkikh wrote:
>> Hi Ferruh,
>>
>>>> +int
>>>> +rte_eth_macsec_select_txsa(uint16_t port_id,
>>>> +			   uint8_t idx, uint8_t an,
>>>> +			   uint32_t pn, uint8_t *key);
>>>> +
>>>>   
>>>>   #include <rte_ethdev_core.h>
>>>>   
>>> These are new ethdev APIs, not driver code, that have been sent after rc1, so
>>> these didn't go through a proper review cycle, we didn't get any comment on any
>>> other possible driver can use it, I am for postponing the series to next release.
>>>
>>> Also there are some mechanical issues [1] but main thing is adding a set of API
>>> to late in release cycle without proper review.
>> I see, that's reasonable.
>>
>> May I suggest another option then: can we do driver only API (almost like ixgbe providing now)?
>> Two points here:
>>
>> 1) Thomas raised a reasonable question whether all the macsec control in general should happen
>> through the rte_security set of APIs. This obviously could be done, but with proper design
>> of rte_security structures and ops.
>>
>> 2) Aquantia is interested in having macsec control code in driver within 19.05, even in form
>> of private driver API as it runs in ixgbe now. This code is functional and will not be
>> changed alot anyway. It could be easily adopted later when point (1) gets a conclusion.
>>
> If there is a commitment to work on a generic solution for 19.08, involving
> other users too, I would be OK to get the support as PMD API for this release.
>
> If that is accepted, please bu sure too add experimental tag to new PMD APIs and
> even add to release notes about intention and that the PMD specific APIs are
> temporary. And if ABI breakage required, put any necessary deprecation notice
> withing this release scope so that the development is not blocked for next release.
>
> Thomas, Andrew, what do you think?

I agree.
  

16/04/2019 11:58, Andrew Rybchenko:
> On 4/16/19 12:43 PM, Ferruh Yigit wrote:
> > On 4/13/2019 8:24 AM, Igor Russkikh wrote:
> >> Hi Ferruh,
> >>
> >>>> +int
> >>>> +rte_eth_macsec_select_txsa(uint16_t port_id,
> >>>> +			   uint8_t idx, uint8_t an,
> >>>> +			   uint32_t pn, uint8_t *key);
> >>>> +
> >>>>   
> >>>>   #include <rte_ethdev_core.h>
> >>>>   
> >>> These are new ethdev APIs, not driver code, that have been sent after rc1, so
> >>> these didn't go through a proper review cycle, we didn't get any comment on any
> >>> other possible driver can use it, I am for postponing the series to next release.
> >>>
> >>> Also there are some mechanical issues [1] but main thing is adding a set of API
> >>> to late in release cycle without proper review.
> >> I see, that's reasonable.
> >>
> >> May I suggest another option then: can we do driver only API (almost like ixgbe providing now)?
> >> Two points here:
> >>
> >> 1) Thomas raised a reasonable question whether all the macsec control in general should happen
> >> through the rte_security set of APIs. This obviously could be done, but with proper design
> >> of rte_security structures and ops.
> >>
> >> 2) Aquantia is interested in having macsec control code in driver within 19.05, even in form
> >> of private driver API as it runs in ixgbe now. This code is functional and will not be
> >> changed alot anyway. It could be easily adopted later when point (1) gets a conclusion.
> >>
> > If there is a commitment to work on a generic solution for 19.08, involving
> > other users too, I would be OK to get the support as PMD API for this release.
> >
> > If that is accepted, please bu sure too add experimental tag to new PMD APIs and
> > even add to release notes about intention and that the PMD specific APIs are
> > temporary. And if ABI breakage required, put any necessary deprecation notice
> > withing this release scope so that the development is not blocked for next release.
> >
> > Thomas, Andrew, what do you think?
> 
> I agree.

+1
  

>>>>> These are new ethdev APIs, not driver code, that have been sent after rc1, so
>>>>> these didn't go through a proper review cycle, we didn't get any comment on any
>>>>> other possible driver can use it, I am for postponing the series to next release.
>>>>>
>>>>> Also there are some mechanical issues [1] but main thing is adding a set of API
>>>>> to late in release cycle without proper review.
>>>> I see, that's reasonable.
>>>>
>>>> May I suggest another option then: can we do driver only API (almost like ixgbe providing now)?
>>>> Two points here:
>>>>
>>>> 1) Thomas raised a reasonable question whether all the macsec control in general should happen
>>>> through the rte_security set of APIs. This obviously could be done, but with proper design
>>>> of rte_security structures and ops.
>>>>
>>>> 2) Aquantia is interested in having macsec control code in driver within 19.05, even in form
>>>> of private driver API as it runs in ixgbe now. This code is functional and will not be
>>>> changed alot anyway. It could be easily adopted later when point (1) gets a conclusion.
>>>>
>>> If there is a commitment to work on a generic solution for 19.08, involving
>>> other users too, I would be OK to get the support as PMD API for this release.
>>>
>>> If that is accepted, please bu sure too add experimental tag to new PMD APIs and
>>> even add to release notes about intention and that the PMD specific APIs are
>>> temporary. And if ABI breakage required, put any necessary deprecation notice
>>> withing this release scope so that the development is not blocked for next release.
>>>
>>> Thomas, Andrew, what do you think?
>>
>> I agree.
> 
> +1

Great, I'll prepare v2 patchset then. Will also start on prototyping possible
rte_security macsec API.

Regards,
  Igor