netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
To: Harald Welte <laforge@gnumonks.org>
Cc: Alexander Lobakin <aleksander.lobakin@intel.com>,
	Wojciech Drewek <wojciech.drewek@intel.com>,
	netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org,
	linux-kernel@vger.kernel.org
Subject: Re: [Intel-wired-lan] [PATCH net-next v6 00/21] ice: add PFCP filter support
Date: Thu, 16 May 2024 12:44:13 +0200	[thread overview]
Message-ID: <ZkXjfeLhB9T5MLfX@mev-dev> (raw)
In-Reply-To: <ZkSivjg7uZsA20ft@nataraja>

On Wed, May 15, 2024 at 01:55:42PM +0200, Harald Welte wrote:
> Daer Alexander, Wojciech,

Hi Harald,

> 
> forgive me for being late to the party, but I just saw the PFCP support
> hitting Linus'' git repo in 1b294a1f35616977caddaddf3e9d28e576a1adbc
> and was trying to figure out what it is all about.  Is there some kind
> of article, kernel documentation or other explanation about it?
> 
> I have a prehistoric background in Linux kernel networking, and have
> been spending much of the last two decades in creating open source
> implemenmtations of 3GPP specifications.
> 
> So I'm very familiar with what PFCP is, and what it does, and how it is
> used as a protocol by the 3GPP control plane to control the user/data
> plane.
> 
> Conceptually it seems very odd to me to have something like *pfcp
> net-devices*.  PFCP is just a control plane protocol, not a tunnel
> mechanism.
> 
> From the Kconfig:
> 
> > +config PFCP
> > +	tristate "Packet Forwarding Control Protocol (PFCP)"
> > +	depends on INET
> > +	select NET_UDP_TUNNEL
> > +	help
> > +	  This allows one to create PFCP virtual interfaces that allows to
> > +	  set up software and hardware offload of PFCP packets.
> 
> I'm curious to understand why are *pfcp* packets hardware offloaded?
> PFCP is just the control plane, similar to you can consider netlink the
> control plane by which userspace programs control the data plane.
> 
> I can fully understand that GTP-U packets are offloaded to kernel space or
> hardware, and that then some control plane mechanism like PFCP is needed
> to control that data plane.  But offloading packets of that control
> protocol?

It is hard for me to answer your concerns, because oposite to you, I
don't have any experience with telco implementations. We had client that
want to add offload rule for PFCP in the same way as for GTP. Intel
hardware support matching on specific PFCP packet parts. We spent some
time looking at possible implementations. As you said, it is a little
odd to follow the same scheme for GTP and PFCP, but it look for me like
reasonable solution.

The main idea is to allow user to match in tc flower on PFCP specific
parts. To do that we need PFCP device (which is like dummy device for
now, it isn't doing anything related to PFCP specification, only parsing).

Do you have better idea for that?

> 
> I also see the following in the patch:
> 
> > +MODULE_DESCRIPTION("Interface driver for PFCP encapsulated traffic");
> 
> PFCP is not an encapsulation protocol for user plane traffic.  It is not
> a tunneling protocol.  GTP-U is the tunneling protocol, whose
> implementations (typically UPFs) are remote-controlled by PFCP.
> 

Agree, it is done like that to simplify implementation and reuse kernel
software stack.

> > +	  Note that this module does not support PFCP protocol in the kernel space.
> > +	  There is no support for parsing any PFCP messages.
> 
> If I may be frank, why do we introduce something called "pfcp" to the
> kernel, if it doesn't actually implement any of the PFCP specification
> 3GPP TS 29.244 (which is specifying a very concrete protocol)?
> 
> Once again, I just try to understand what you're trying to do here. It's
> very much within my professional field, but I somehow cannot align what
> I see within this patch set with my existing world view of what PFCP is
> and how it works.
> 
> If anyone else has a better grasp of the architecture of this kernel
> PFCP support, or has any pointers, I'd be very happy to follow up
> on that.

This is the way to allow user to steer PFCP packet based on specific
opts (type and seid) using tc flower. If you have better solution for
that I will probably agree with you and will be willing to help you
with better implementation.

I assume the biggest problem here is with treating PFCP as a tunnel
(done for simplification and reuse) and lack of any functionality of
PFCP device (moving the PFCP specification implementation into kernel
probably isn't good idea and may never be accepted).

Offloading doesn't sound like problematic. If there is a user that want
to use that (to offload for better performance, or even to steer between
VFs based on PFCP specific parts) why not allow to do that?

In your opinion there should not be the pfcp device in kernel, or the
device should have more functionality (or any functionality, because now
it is only parsing)?

> 
> Thanks for your time,
> 	Harald
>

Thanks,
Michal

> -- 
> - Harald Welte <laforge@gnumonks.org>          https://laforge.gnumonks.org/
> ============================================================================
> "Privacy in residential applications is a desirable marketing option."
>                                                   (ETSI EN 300 175-7 Ch. A6)

  reply	other threads:[~2024-05-16 10:44 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-27 15:23 [PATCH net-next v6 00/21] ice: add PFCP filter support Alexander Lobakin
2024-03-27 15:23 ` [PATCH net-next v6 01/21] lib/bitmap: add bitmap_{read,write}() Alexander Lobakin
2024-03-27 15:23 ` [PATCH net-next v6 02/21] lib/test_bitmap: add tests for bitmap_{read,write}() Alexander Lobakin
2024-03-27 15:47   ` Andy Shevchenko
2024-03-27 16:49     ` Alexander Lobakin
2024-03-27 15:23 ` [PATCH net-next v6 03/21] lib/test_bitmap: use pr_info() for non-error messages Alexander Lobakin
2024-03-27 15:23 ` [PATCH net-next v6 04/21] bitops: add missing prototype check Alexander Lobakin
2024-03-27 15:23 ` [PATCH net-next v6 05/21] bitops: make BYTES_TO_BITS() treewide-available Alexander Lobakin
2024-03-27 15:23 ` [PATCH net-next v6 06/21] bitops: let the compiler optimize {__,}assign_bit() Alexander Lobakin
2024-03-27 15:23 ` [PATCH net-next v6 07/21] linkmode: convert linkmode_{test,set,clear,mod}_bit() to macros Alexander Lobakin
2024-03-27 15:23 ` [PATCH net-next v6 08/21] s390/cio: rename bitmap_size() -> idset_bitmap_size() Alexander Lobakin
2024-03-27 15:23 ` [PATCH net-next v6 09/21] fs/ntfs3: add prefix to bitmap_size() and use BITS_TO_U64() Alexander Lobakin
2024-03-27 15:23 ` [PATCH net-next v6 10/21] btrfs: rename bitmap_set_bits() -> btrfs_bitmap_set_bits() Alexander Lobakin
2024-03-27 15:23 ` [PATCH net-next v6 11/21] tools: move alignment-related macros to new <linux/align.h> Alexander Lobakin
2024-03-27 15:23 ` [PATCH net-next v6 12/21] bitmap: introduce generic optimized bitmap_size() Alexander Lobakin
2024-03-27 15:23 ` [PATCH net-next v6 13/21] bitmap: make bitmap_{get,set}_value8() use bitmap_{read,write}() Alexander Lobakin
2024-05-29 15:12   ` Robin Murphy
2024-05-30 17:11     ` Yury Norov
2024-05-30 17:50       ` Robin Murphy
2024-03-27 15:23 ` [PATCH net-next v6 14/21] lib/bitmap: add compile-time test for __assign_bit() optimization Alexander Lobakin
2024-03-27 15:23 ` [PATCH net-next v6 15/21] ip_tunnel: use a separate struct to store tunnel params in the kernel Alexander Lobakin
2024-04-04 14:24   ` Dan Carpenter
2024-04-04 15:47     ` Alexander Lobakin
2024-03-27 15:23 ` [PATCH net-next v6 16/21] ip_tunnel: convert __be16 tunnel flags to bitmaps Alexander Lobakin
2024-03-27 15:23 ` [PATCH net-next v6 17/21] net: net_test: add tests for IP tunnel flags conversion helpers Alexander Lobakin
2024-03-27 15:23 ` [PATCH net-next v6 18/21] pfcp: add PFCP module Alexander Lobakin
2024-03-27 15:23 ` [PATCH net-next v6 19/21] pfcp: always set pfcp metadata Alexander Lobakin
2024-04-03 20:59   ` Arnd Bergmann
2024-04-04  9:45     ` Michal Swiatkowski
2024-04-04  9:56       ` Arnd Bergmann
2024-04-04 10:12         ` [Intel-wired-lan] " Michal Swiatkowski
2024-03-27 15:23 ` [PATCH net-next v6 20/21] ice: refactor ICE_TC_FLWR_FIELD_ENC_OPTS Alexander Lobakin
2024-03-27 15:23 ` [PATCH net-next v6 21/21] ice: Add support for PFCP hardware offload in switchdev Alexander Lobakin
2024-04-01 10:00 ` [PATCH net-next v6 00/21] ice: add PFCP filter support patchwork-bot+netdevbpf
2024-05-15 11:55 ` Harald Welte
2024-05-16 10:44   ` Michal Swiatkowski [this message]
2024-05-16 21:30     ` [Intel-wired-lan] " Harald Welte
2024-05-17 14:01       ` Michal Swiatkowski

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=ZkXjfeLhB9T5MLfX@mev-dev \
    --to=michal.swiatkowski@linux.intel.com \
    --cc=aleksander.lobakin@intel.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=laforge@gnumonks.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=wojciech.drewek@intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).