Re: [PATCH net] mlxsw: spectrum_ptp: Fix napi_gro_receive() call from GC workqueue context

[Petr Machata <petrm@nvidia.com>]

Taylor Bates <tmbates12@gmail.com> writes:

> This patch adds an ingress-delivery helper for the PTP packet_finish()
> path that calls netif_receive_skb() instead of napi_gro_receive().
> netif_receive_skb(), unlike napi_gro_receive(), can be called from outside
> of the NAPI instance's poll context, which can occur at the call site for
> this path. RX stats accounting and the skb->dev assignment are still
> preserved; the only change is the delivery call itself.
>
> This removes GRO batching for any PTP event traffic received by the mlxsw
> trap, but given the relatively low volume of traffic characteristic of the
> protocol, and impact limited to only Spectrum-1 ASICs, this is an
> acceptable solution.
>
> Fixes: 1ba06ca96ca2 ("mlxsw: Switch to napi_gro_receive()")
> Signed-off-by: Taylor Bates <tmbates12@gmail.com>

Reviewed-by: Petr Machata <petrm@nvidia.com>

> ---
>  .../ethernet/mellanox/mlxsw/spectrum_ptp.c    | 34 ++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
> index 9939749c47bc..9c5862f4e16a 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
> @@ -572,6 +572,38 @@ mlxsw_sp1_ptp_unmatched_remove(struct mlxsw_sp *mlxsw_sp,

> +static void mlxsw_sp1_ptp_rx_finish(struct mlxsw_sp_port *mlxsw_sp_port,
> +				    struct sk_buff *skb)
> +{
> +	struct mlxsw_sp_port_pcpu_stats *pcpu_stats;
> +
> +	skb->dev = mlxsw_sp_port->dev;
> +
> +	pcpu_stats = this_cpu_ptr(mlxsw_sp_port->pcpu_stats);
> +	u64_stats_update_begin(&pcpu_stats->syncp);
> +	pcpu_stats->rx_packets++;
> +	pcpu_stats->rx_bytes += skb->len;
> +	u64_stats_update_end(&pcpu_stats->syncp);
> +
> +	skb->protocol = eth_type_trans(skb, skb->dev);
> +	netif_receive_skb(skb);
> +}

Not entirely happy with inlining like this, but there are at least two
copies of this block already. We could factor out the commonality at
some point, but the code is mechanical and I think it's OK.

> +
>  /* This function is called in the following scenarios:
>   *
>   * 1) When a packet is matched with its timestamp.