Re: [RFC PATCH iwl-next 4/6] pfcp: always set pfcp metadata

[Simon Horman <simon.horman@corigine.com>]

On Thu, Jun 01, 2023 at 03:19:27PM +0200, Marcin Szycik wrote:
> From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> 
> In PFCP receive path set metadata needed by flower code to do correct
> classification based on this metadata.
> 
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> Signed-off-by: Marcin Szycik <marcin.szycik@linux.intel.com>

A few minor nits below.
Otherwise, looks good to me.

Reviewed-by: Simon Horman <simon.horman@corigine.com>

...

> @@ -30,6 +33,82 @@ struct pfcp_net {
>  	struct list_head	pfcp_dev_list;
>  };
>  
> +static int
> +pfcp_session_recv(struct pfcp_dev *pfcp, struct sk_buff *skb,
> +		  struct pfcp_metadata *md)
> +{
> +	struct pfcphdr_session *unparsed = pfcp_hdr_session(skb);
> +
> +	md->seid = unparsed->seid;
> +	md->type = PFCP_TYPE_SESSION;
> +
> +	return 0;
> +}

nit: This the return type of this function could be void.

> +
> +static int
> +pfcp_node_recv(struct pfcp_dev *pfcp, struct sk_buff *skb,
> +	       struct pfcp_metadata *md)
> +{
> +	md->type = PFCP_TYPE_NODE;
> +
> +	return 0;
> +}

Ditto.

> +
> +static int pfcp_encap_recv(struct sock *sk, struct sk_buff *skb)
> +{
> +	IP_TUNNEL_DECLARE_FLAGS(flags) = { };
> +	struct metadata_dst *tun_dst;
> +	struct pfcp_metadata *md;
> +	struct pfcphdr *unparsed;
> +	struct pfcp_dev *pfcp;
> +
> +	if (unlikely(!pskb_may_pull(skb, PFCP_HLEN)))
> +		goto drop;
> +
> +	pfcp = rcu_dereference_sk_user_data(sk);
> +	if (unlikely(!pfcp))
> +		goto drop;
> +
> +	unparsed = pfcp_hdr(skb);
> +
> +	bitmap_zero(flags, __IP_TUNNEL_FLAG_NUM);
> +	tun_dst = udp_tun_rx_dst(skb, sk->sk_family, flags, 0,
> +				 sizeof(*md));
> +	if (unlikely(!tun_dst))
> +		goto drop;
> +
> +	md = ip_tunnel_info_opts(&tun_dst->u.tun_info);
> +	if (unlikely(!md))
> +		goto drop;
> +
> +	if (unparsed->flags & PFCP_SEID_FLAG)
> +		pfcp_session_recv(pfcp, skb, md);
> +	else
> +		pfcp_node_recv(pfcp, skb, md);
> +
> +	__set_bit(IP_TUNNEL_PFCP_OPT_BIT, flags);
> +	ip_tunnel_info_opts_set(&tun_dst->u.tun_info, md, sizeof(*md),
> +				flags);
> +
> +	if (unlikely(iptunnel_pull_header(skb, PFCP_HLEN, skb->protocol,
> +					  !net_eq(sock_net(sk),
> +					  dev_net(pfcp->dev)))))
> +		goto drop;
> +
> +	skb_dst_set(skb, (struct dst_entry *)tun_dst);
> +
> +	skb_reset_network_header(skb);
> +	skb_reset_mac_header(skb);
> +	skb->dev = pfcp->dev;
> +
> +	gro_cells_receive(&pfcp->gro_cells, skb);
> +
> +	return 0;
> +drop:
> +	kfree_skb(skb);
> +	return 0;
> +}

...