netdev - Re: [PATCH iwl-next v3 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, 24 Jul 2023 15:19:41 +0200
From: Marcin Szycik <marcin.szycik@...ux.intel.com>
To: Andy Shevchenko <andy@...nel.org>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
 wojciech.drewek@...el.com, michal.swiatkowski@...ux.intel.com,
 aleksander.lobakin@...el.com, davem@...emloft.net, kuba@...nel.org,
 jiri@...nulli.us, pabeni@...hat.com, jesse.brandeburg@...el.com,
 simon.horman@...igine.com, idosch@...dia.com
Subject: Re: [PATCH iwl-next v3 4/6] pfcp: always set pfcp metadata



On 21.07.2023 17:02, Andy Shevchenko wrote:
> On Fri, Jul 21, 2023 at 09:15:30AM +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.
> 
> ...
> 
> + bits.h
> + types.h

Will add.

> 
>> +#include <net/udp_tunnel.h>
>> +#include <net/dst_metadata.h>
>> +
>>  #define PFCP_PORT 8805
>>  
>> +/* PFCP protocol header */
>> +struct pfcphdr {
>> +	u8	flags;
>> +	u8	message_type;
>> +	__be16	message_length;
>> +};
>> +
>> +/* PFCP header flags */
>> +#define PFCP_SEID_FLAG		BIT(0)
>> +#define PFCP_MP_FLAG		BIT(1)
>> +
>> +#define PFCP_VERSION_SHIFT	5
>> +#define PFCP_VERSION_MASK	((1 << PFCP_VERSION_SHIFT) - 1)
> 
> GENMASK() since you already use BIT()

Will change.

> 
>> +#define PFCP_HLEN (sizeof(struct udphdr) + sizeof(struct pfcphdr))
>> +
>> +/* PFCP node related messages */
>> +struct pfcphdr_node {
>> +	u8	seq_number[3];
>> +	u8	reserved;
>> +};
>> +
>> +/* PFCP session related messages */
>> +struct pfcphdr_session {
>> +	__be64	seid;
>> +	u8	seq_number[3];
>> +#ifdef __LITTLE_ENDIAN_BITFIELD
>> +	u8	message_priority:4,
>> +		reserved:4;
>> +#elif defined(__BIG_ENDIAN_BITFIELD)
>> +	u8	reserved:4,
>> +		message_priprity:4;
>> +#else
>> +#error "Please fix <asm/byteorder>"
>> +#endif
>> +};
>> +
>> +struct pfcp_metadata {
>> +	u8 type;
>> +	__be64 seid;
>> +} __packed;
>> +
>> +enum {
>> +	PFCP_TYPE_NODE		= 0,
>> +	PFCP_TYPE_SESSION	= 1,
>> +};
> 
> ...
> 
>> +/* IP header + UDP + PFCP + Ethernet header */
>> +#define PFCP_HEADROOM (20 + 8 + 4 + 14)
> 
> Instead of comment like above, just use defined sizes.
> 
>> +/* IPv6 header + UDP + PFCP + Ethernet header */
>> +#define PFCP6_HEADROOM (40 + 8 + 4 + 14)

Will change.

> 
> sizeof(ipv6hdr)
> sizeof(updhdr)
> ...
> 
> Don't forget to include respective headers.
> 

Thank you for review!
Marcin