All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marcin Szycik <marcin.szycik@linux.intel.com>
To: Andy Shevchenko <andy@kernel.org>
Cc: jiri@resnulli.us, netdev@vger.kernel.org, idosch@nvidia.com,
	jesse.brandeburg@intel.com, intel-wired-lan@lists.osuosl.org,
	kuba@kernel.org, simon.horman@corigine.com, pabeni@redhat.com,
	davem@davemloft.net
Subject: Re: [Intel-wired-lan] [PATCH iwl-next v3 4/6] pfcp: always set pfcp metadata
Date: Mon, 24 Jul 2023 15:19:41 +0200	[thread overview]
Message-ID: <9c3da951-7a08-2b97-309c-e7939703d11c@linux.intel.com> (raw)
In-Reply-To: <ZLqeB/0aoe6GQUVi@smile.fi.intel.com>



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@linux.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
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

WARNING: multiple messages have this Message-ID (diff)
From: Marcin Szycik <marcin.szycik@linux.intel.com>
To: Andy Shevchenko <andy@kernel.org>
Cc: intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
	wojciech.drewek@intel.com, michal.swiatkowski@linux.intel.com,
	aleksander.lobakin@intel.com, davem@davemloft.net,
	kuba@kernel.org, jiri@resnulli.us, pabeni@redhat.com,
	jesse.brandeburg@intel.com, simon.horman@corigine.com,
	idosch@nvidia.com
Subject: Re: [PATCH iwl-next v3 4/6] pfcp: always set pfcp metadata
Date: Mon, 24 Jul 2023 15:19:41 +0200	[thread overview]
Message-ID: <9c3da951-7a08-2b97-309c-e7939703d11c@linux.intel.com> (raw)
In-Reply-To: <ZLqeB/0aoe6GQUVi@smile.fi.intel.com>



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@linux.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

  reply	other threads:[~2023-07-24 13:26 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-21  7:15 [Intel-wired-lan] [PATCH iwl-next v3 0/6] ice: Add PFCP filter support Marcin Szycik
2023-07-21  7:15 ` Marcin Szycik
2023-07-21  7:15 ` [Intel-wired-lan] [PATCH iwl-next v3 1/6] ip_tunnel: use a separate struct to store tunnel params in the kernel Marcin Szycik
2023-07-21  7:15   ` Marcin Szycik
2023-07-21 14:21   ` [Intel-wired-lan] " Andy Shevchenko
2023-07-21 14:21     ` Andy Shevchenko
2023-07-24 14:21     ` [Intel-wired-lan] " Alexander Lobakin
2023-07-24 14:21       ` Alexander Lobakin
2023-07-21  7:15 ` [Intel-wired-lan] [PATCH iwl-next v3 2/6] ip_tunnel: convert __be16 tunnel flags to bitmaps Marcin Szycik
2023-07-21  7:15   ` Marcin Szycik
     [not found]   ` <ZLqZRFa1VOHHWCqX@smile.fi.intel.com>
2023-07-26 11:09     ` [Intel-wired-lan] " Alexander Lobakin
2023-07-26 11:09       ` Alexander Lobakin
2023-07-26 12:01       ` [Intel-wired-lan] " Andy Shevchenko
2023-07-26 12:01         ` Andy Shevchenko
2023-07-26 12:05         ` [Intel-wired-lan] " Andy Shevchenko
2023-07-26 12:05           ` Andy Shevchenko
2023-07-26 13:16         ` [Intel-wired-lan] " Alexander Lobakin
2023-07-26 13:16           ` Alexander Lobakin
2023-07-26 14:32           ` [Intel-wired-lan] " Andy Shevchenko
2023-07-26 14:32             ` Andy Shevchenko
2023-07-21  7:15 ` [Intel-wired-lan] [PATCH iwl-next v3 3/6] pfcp: add PFCP module Marcin Szycik
2023-07-21  7:15   ` Marcin Szycik
2023-07-21 14:54   ` [Intel-wired-lan] " Andy Shevchenko
2023-07-21 14:54     ` Andy Shevchenko
2023-07-24 10:36     ` [Intel-wired-lan] " Marcin Szycik
2023-07-24 10:36       ` Marcin Szycik
2023-07-21  7:15 ` [Intel-wired-lan] [PATCH iwl-next v3 4/6] pfcp: always set pfcp metadata Marcin Szycik
2023-07-21  7:15   ` Marcin Szycik
2023-07-21 15:02   ` [Intel-wired-lan] " Andy Shevchenko
2023-07-21 15:02     ` Andy Shevchenko
2023-07-24 13:19     ` Marcin Szycik [this message]
2023-07-24 13:19       ` Marcin Szycik
2023-07-21  7:15 ` [Intel-wired-lan] [PATCH iwl-next v3 5/6] ice: refactor ICE_TC_FLWR_FIELD_ENC_OPTS Marcin Szycik
2023-07-21  7:15   ` Marcin Szycik
2023-07-21  7:15 ` [Intel-wired-lan] [PATCH iwl-next v3 6/6] ice: Add support for PFCP hardware offload in switchdev Marcin Szycik
2023-07-21  7:15   ` Marcin Szycik
2023-07-21 15:07   ` [Intel-wired-lan] " Andy Shevchenko
2023-07-21 15:07     ` Andy Shevchenko
2023-07-24 13:58     ` [Intel-wired-lan] " Marcin Szycik
2023-07-24 13:58       ` Marcin Szycik
2023-07-24 14:10       ` [Intel-wired-lan] " Andy Shevchenko
2023-07-24 14:10         ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9c3da951-7a08-2b97-309c-e7939703d11c@linux.intel.com \
    --to=marcin.szycik@linux.intel.com \
    --cc=andy@kernel.org \
    --cc=davem@davemloft.net \
    --cc=idosch@nvidia.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jesse.brandeburg@intel.com \
    --cc=jiri@resnulli.us \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=simon.horman@corigine.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.