2023-08-28, 16:46:02 +0300, Radu Pirea (OSS) wrote:
>
>
> On 28.08.2023 13:17, Sabrina Dubroca wrote:
> > 2023-08-24, 12:16:15 +0300, Radu Pirea (NXP OSS) wrote:
> > > Implement mdo_insert_tx_tag to insert the TLV header in the ethernet
> > > frame.
> > >
> > > If extscs parameter is set to 1, then the TLV header will contain the
> > > TX SC that will be used to encrypt the frame, otherwise the TX SC will
> > > be selected using the MAC source address.
> >
> > In which case would a user choose not to use the SCI? Using the MAC
> > address is probably fine in basic setups, but having to fiddle with a
> > module parameter (so unloading and reloading the module, which means
> > losing network connectivity) to make things work when the setup
> > evolves is really not convenient.
> >
> > Is there a drawback to always using the SCI?
> >
>
> I see your concern. If the PHY driver is reloaded, then the offloaded MACsec
> configuration will vanish from the hardware. Actually, just a call to
> phy_disconnect is enough to break an offloaded MACsec iface and can be
> achieved by:
> ip link set eth0 down && ip link set eth0 up
And it's not restored when the link goes back up? That's inconvenient :/
Do we end up with inconsistent state? ie driver and core believe
everything is still offloaded, but HW lost all state? do we leak
some resources allocated by the driver?
We could add a flush/restore in macsec_notify when the lower device
goes down/up, maybe limited to devices that request this (I don't know
if all devices would need it, or maybe all devices offloading to the
PHY but not to the MAC).
And what happens in this case?
ip link add link eth0 type macsec offload phy
ip link set eth0 down
ip macsec add macsec0 rx sci ...
ip macsec add macsec0 tx sa 0 ...
# etc
ip link set eth0 up
Will offload work with the current code?
> The only drawback is related to the PTP frames encryption. Due to hardware
> limitations, PHY timestamping + MACsec will not work if the custom header is
> inserted. The only way to get this work is by using the MAC SA selection and
> running PTP on the real netdev.
Could you add some documentation explaining that? Users need this
information to make the right choice for their use case. Maybe
directly in the description of the module parameter, something like:
"Select the TX SC using TLV header information. PTP frames encryption
cannot work when this feature is enabled."
If it's in the module parameter I guess it can't be too
verbose. Otherwise I don't know where else to put it.
And the parameter's name and/or description should probably include
macsec/MACsec if it's visible at the level of the whole module (ie if
macsec support isn't a separate module), just to give context at to
what the TXSC is (and what the encryption for the PTP frames refers
to).
--
Sabrina
On Wed, 2023-08-30 at 13:35 +0200, Sabrina Dubroca wrote:
...
> And it's not restored when the link goes back up? That's inconvenient
> :/
> Do we end up with inconsistent state? ie driver and core believe
> everything is still offloaded, but HW lost all state? do we leak
> some resources allocated by the driver?
Yes. We end up with inconsistent state. The HW will lost all state when
the phy is reseted. No resource is leaked, everything is there, but the
configuration needs to be reapplied.
>
> We could add a flush/restore in macsec_notify when the lower device
> goes down/up, maybe limited to devices that request this (I don't
> know
> if all devices would need it, or maybe all devices offloading to the
> PHY but not to the MAC).
Agreed.
We can do a flush very simple, but to restore the configuration maybe
we should to save the key in the macsec_key structure. I am not sure if
the key can be extracted from crypto_aead structure.
>
> And what happens in this case?
> ip link add link eth0 type macsec offload phy
> ip link set eth0 down
> ip macsec add macsec0 rx sci ...
> ip macsec add macsec0 tx sa 0 ...
> # etc
> ip link set eth0 up
>
> Will offload work with the current code?
(the interface was up before)
[root@alarm ~]# ip link add link end0 macsec0 type macsec encrypt on
offload phy
[root@alarm ~]# ip link set end0 down
[root@alarm ~]# ip macsec add macsec0 rx port 1 address
00:01:be:be:ef:33
RTNETLINK answers: Operation not supported
But let's consider the next case:
ip link add link eth0 type macsec offload phy
ip link set eth0 down
ip link set eth0 up
ip macsec add macsec0 rx sci ...
ip macsec add macsec0 tx sa 0 ...
# etc
In this case, any HW configuration written by .mdo_add_secy will be
lost.
>
> > The only drawback is related to the PTP frames encryption. Due to
> > hardware
> > limitations, PHY timestamping + MACsec will not work if the custom
> > header is
> > inserted. The only way to get this work is by using the MAC SA
> > selection and
> > running PTP on the real netdev.
>
> Could you add some documentation explaining that? Users need this
> information to make the right choice for their use case. Maybe
> directly in the description of the module parameter, something like:
> "Select the TX SC using TLV header information. PTP frames encryption
> cannot work when this feature is enabled."
>
> If it's in the module parameter I guess it can't be too
> verbose. Otherwise I don't know where else to put it.
>
> And the parameter's name and/or description should probably include
> macsec/MACsec if it's visible at the level of the whole module (ie if
> macsec support isn't a separate module), just to give context at to
> what the TXSC is (and what the encryption for the PTP frames refers
> to).
I will improve the comment and change the name. Thank you.