| Message ID | 20240118191811.50271-2-rrameshbabu@nvidia.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2,1/2] Revert "net: macsec: use skb_ensure_writable_head_tail to expand the skb" | expand |
2024-01-18, 11:18:07 -0800, Rahul Rameshbabu wrote: > A number of MACsec offload implementers do not implement > .mdo_insert_tx_tag. These implementers also do not specify the needed > headroom/tailroom and depend on the default in the core MACsec stack. FWIW, I had the same concern when Radu submitted these changes, and he answered that the extra room was only needed for SW, not for offload: https://lore.kernel.org/all/a5ef22bc-2457-5eef-7cff-529711c5c242@oss.nxp.com/ I'm not really objecting to this patch, but is it fixing a bug? If so, it would be useful to describe the problem you're seeing in the commit message for this change. Does your driver require the default headroom/tailroom? > Fixes: a73d8779d61a ("net: macsec: introduce mdo_insert_tx_tag") > Cc: Radu Pirea (NXP OSS) <radu-nicolae.pirea@oss.nxp.com> > Cc: David S. Miller <davem@davemloft.net> > Cc: Sabrina Dubroca <sd@queasysnail.net> > Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
On Sun, 21 Jan, 2024 10:29:08 +0100 Sabrina Dubroca <sd@queasysnail.net> wrote: > 2024-01-18, 11:18:07 -0800, Rahul Rameshbabu wrote: >> A number of MACsec offload implementers do not implement >> .mdo_insert_tx_tag. These implementers also do not specify the needed >> headroom/tailroom and depend on the default in the core MACsec stack. > > FWIW, I had the same concern when Radu submitted these changes, and he > answered that the extra room was only needed for SW, not for offload: > https://lore.kernel.org/all/a5ef22bc-2457-5eef-7cff-529711c5c242@oss.nxp.com/ Thanks, this conversation was helpful. > > I'm not really objecting to this patch, but is it fixing a bug? If so, > it would be useful to describe the problem you're seeing in the commit > message for this change. It isn't. I was not sure if other drivers depended on the default headroom/tailroom set by the MACsec stack. > > Does your driver require the default headroom/tailroom? > mlx5 does not. I am very open to dropping this patch in the series. -- Thanks, Rahul Rameshbabu
diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c index 7f5426285c61..907c15f0fb14 100644 --- a/drivers/net/macsec.c +++ b/drivers/net/macsec.c @@ -2601,7 +2601,7 @@ static void macsec_set_head_tail_room(struct net_device *dev) const struct macsec_ops *ops; ops = macsec_get_ops(macsec, NULL); - if (ops) { + if (ops && ops->mdo_insert_tx_tag) { needed_headroom = ops->needed_headroom; needed_tailroom = ops->needed_tailroom; } else {
A number of MACsec offload implementers do not implement .mdo_insert_tx_tag. These implementers also do not specify the needed headroom/tailroom and depend on the default in the core MACsec stack. Fixes: a73d8779d61a ("net: macsec: introduce mdo_insert_tx_tag") Cc: Radu Pirea (NXP OSS) <radu-nicolae.pirea@oss.nxp.com> Cc: David S. Miller <davem@davemloft.net> Cc: Sabrina Dubroca <sd@queasysnail.net> Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com> --- Notes: Changes: v1->v2: * Introduce new commit * Fix headroom/tailroom calculation when MACsec offload implementer does not implement .mdo_insert_tx_tag drivers/net/macsec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)