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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 5 Jun 2023 11:34:27 +0200
From: Simon Horman <simon.horman@...igine.com>
To: Marcin Szycik <marcin.szycik@...ux.intel.com>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
	wojciech.drewek@...el.com, michal.swiatkowski@...ux.intel.com,
	alexandr.lobakin@...el.com, davem@...emloft.net, kuba@...nel.org,
	jiri@...nulli.us, pabeni@...hat.com, jesse.brandeburg@...el.com,
	idosch@...dia.com
Subject: Re: [RFC PATCH iwl-next 4/6] pfcp: always set pfcp metadata

On Thu, Jun 01, 2023 at 03:19:27PM +0200, Marcin Szycik wrote:
> From: Michal Swiatkowski <michal.swiatkowski@...ux.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@...ux.intel.com>
> Signed-off-by: Marcin Szycik <marcin.szycik@...ux.intel.com>

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

Reviewed-by: Simon Horman <simon.horman@...igine.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;
> +}

...