All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH net-next 0/3] support "flow-based" datapath in l2tp
@ 2021-09-29  9:45 Tom Parkin
  2021-09-29  9:45 ` [RFC PATCH net-next 1/3] net/l2tp: add virtual tunnel device Tom Parkin
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Tom Parkin @ 2021-09-29  9:45 UTC (permalink / raw)
  To: netdev; +Cc: jchapman, Tom Parkin

The traditional l2tp datapath in the kernel allocates of a netdev for
each l2tp session.  For larger session populations this limits
scalability.

Other protocols (such as geneve) support a mode whereby a single virtual
netdev is used to manage packetflows for multiple logical sessions: a
much more scalable solution.

This RFC patch series extends l2tp to support this mode of operation:

    * On creation of a tunnel instance a new tunnel virtual device is
      created (in this patch series it is named according to its ID for
      ease of testing, but this is potentially racy: alternatives are
      mentioned in the code comments).

    * For l2tp encapsulation, tc rules can be added to redirect traffic
      to the virtual tunnel device, e.g.

            tc qdisc add dev eth0 handle ffff: ingress
            tc filter add dev eth0 \
                    parent ffff: \
                    matchall \
                    action tunnel_key set \
                            src_ip 0.0.0.1 \
                            dst_ip 0.0.0.1 \
                            id 1 \
                    action mirred egress redirect dev l2tpt1

      This series utilises the 'id' parameter to refer to session ID
      within the tunnel, and the src_ip/dst_ip parameters are ignored.

    * For l2tp decapsulation, a new session data path is implemented.

      On receipt of an l2tp data packet on the tunnel socket, the l2tp
      headers are removed as normal, and the session ID of the target
      session associated with the skb using ip tunnel dst metadata.

      The skb is then redirected to the tunnel virtual netdev: tc rules
      can then be added to match traffic based on the session ID and
      redirect it to the correct interface:

            tc qdisc add dev l2tpt1 handle ffff: ingress
            tc filter add dev l2tpt1 \
                    parent ffff: \
                    flower enc_key_id 1 \
                    action mirred egress redirect dev eth0

      In the case that no tc rule matches an incoming packet, the tunnel
      virtual device implements an rx handler which swallows the packet
      in order to prevent it continuing through the network stack.

I welcome any comments on:

    1. Whether this RFC represents a good approach for improving
       the l2tp datapath?

    2. Architectural/design feedback on this implementation.

The code here isn't production-ready by any means, although any comments
on bugs or other issues with the series as it stands are also welcome.

Tom Parkin (3):
  net/l2tp: add virtual tunnel device
  net/l2tp: add flow-based session create API
  net/l2tp: add netlink attribute to enable flow-based session creation

 include/uapi/linux/l2tp.h |   1 +
 net/l2tp/l2tp_core.c      | 208 ++++++++++++++++++++++++++++++++++++++
 net/l2tp/l2tp_core.h      |   9 ++
 net/l2tp/l2tp_netlink.c   |  36 ++++---
 4 files changed, 241 insertions(+), 13 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [RFC PATCH net-next 1/3] net/l2tp: add virtual tunnel device
  2021-09-29  9:45 [RFC PATCH net-next 0/3] support "flow-based" datapath in l2tp Tom Parkin
@ 2021-09-29  9:45 ` Tom Parkin
  2021-09-29  9:45 ` [RFC PATCH net-next 2/3] net/l2tp: add flow-based session create API Tom Parkin
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Tom Parkin @ 2021-09-29  9:45 UTC (permalink / raw)
  To: netdev; +Cc: jchapman, Tom Parkin

The traditional L2TP datapath in the kernel requires allocation of a
netdev for each L2TP session.  For larger populations this limits
scalability.

Other protocols (such as geneve) support a mode whereby a single virtual
netdev is used to manage packetflows for multiple logical sessions: a
much more scalable solution.

Extend l2tp to offer a tunnel network device to support this mode of
operation.

l2tp_core.c keeps track of a netdev per tunnel.  The device is allocated
on registration of the tunnel and is named using the local tunnel ID.

Traffic may then be sent over the tunnel socket using dst_metadata set
by e.g. tc's tunnel_key action.  This PoC patch uses the 'id' field to
represent the ID of the session inside the tunnel.

For example:

        # Create a UDP encap tunnel to a peer, and create a session
        # inside the tunnel
        ip l2tp add tunnel \
                remote 192.168.178.30 \
                local 192.168.122.35 \
                tunne1_id 1 \
                peer_tunnel_id 1 \
                encap udp \
                udp_sport 1701 \
                udp_dport 1701
        ip l2tp add session \
                tunnel_id 1 \
                session_id 1 \
                peer_session_id 1
        ip link set dev l2tpt1 up

        # Add tc flower rule to send all eth0 packets over the tunnel:
        # the src_ip and dst_ip parameters here are dummy data ignored
        # by the L2TP data path
        tc qdisc add dev eth0 handle ffff: ingress
        tc filter add dev eth0 \
                parent ffff: \
                matchall \
                action tunnel_key set \
                        src_ip 0.0.0.1 \
                        dst_ip 0.0.0.1 \
                        id 1 \
                action mirred egress redirect dev l2tpt1

Signed-off-by: Tom Parkin <tparkin@katalix.com>
---
 net/l2tp/l2tp_core.c | 162 +++++++++++++++++++++++++++++++++++++++++++
 net/l2tp/l2tp_core.h |   1 +
 2 files changed, 163 insertions(+)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 93271a2632b8..6a4d3d785c65 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -56,6 +56,7 @@
 #include <net/inet_ecn.h>
 #include <net/ip6_route.h>
 #include <net/ip6_checksum.h>
+#include <net/dst_metadata.h>
 
 #include <asm/byteorder.h>
 #include <linux/atomic.h>
@@ -101,6 +102,10 @@ struct l2tp_skb_cb {
 
 static struct workqueue_struct *l2tp_wq;
 
+struct l2tp_tunl_dev_priv {
+	u32 tunnel_id;
+};
+
 /* per-net private data for this module */
 static unsigned int l2tp_net_id;
 struct l2tp_net {
@@ -1248,6 +1253,27 @@ static void l2tp_tunnel_del_work(struct work_struct *work)
 		}
 	}
 
+	/* If the tunnel has a netdev, release it now.
+	 *
+	 * FIXME: feels like a race condition in the making:
+	 * our alloc_netdev call uses an interface name derived from
+	 * tunnel ID.  Both tunnel ID and interface name must be
+	 * unique, and hence if there's a lag between the tunnel
+	 * removal from the list below and the netdev going away,
+	 * there could be allocation failures.
+	 *
+	 * An alternative scheme might be to let alloc_netdev
+	 * auto-assign the tunnel device name: in this case we would
+	 * need to indicate that name back to userspace using netlink.
+	 *
+	 * Another approach would be to have the netlink tunnel
+	 * create command call out the tunnel device name.
+	 */
+	rtnl_lock();
+	if (tunnel->dev)
+		unregister_netdevice(tunnel->dev);
+	rtnl_unlock();
+
 	/* Remove the tunnel struct from the tunnel list */
 	pn = l2tp_pernet(tunnel->l2tp_net);
 	spin_lock_bh(&pn->l2tp_tunnel_list_lock);
@@ -1452,6 +1478,7 @@ static int l2tp_validate_socket(const struct sock *sk, const struct net *net,
 	return 0;
 }
 
+static struct net_device *l2tp_tunnel_netdev_alloc(struct net *net, u32 tunnel_id);
 int l2tp_tunnel_register(struct l2tp_tunnel *tunnel, struct net *net,
 			 struct l2tp_tunnel_cfg *cfg)
 {
@@ -1520,6 +1547,8 @@ int l2tp_tunnel_register(struct l2tp_tunnel *tunnel, struct net *net,
 	if (tunnel->fd >= 0)
 		sockfd_put(sock);
 
+	tunnel->dev = l2tp_tunnel_netdev_alloc(net, tunnel->tunnel_id);
+
 	return 0;
 
 err_sock:
@@ -1632,6 +1661,139 @@ struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunn
 }
 EXPORT_SYMBOL_GPL(l2tp_session_create);
 
+/*****************************************************************************
+ * Tunnel virtual netdev
+ *****************************************************************************/
+
+static int l2tp_tunl_dev_init(struct net_device *dev)
+{
+	netdev_lockdep_set_classes(dev);
+	return 0;
+}
+
+static void l2tp_tunl_dev_uninit(struct net_device *dev)
+{
+}
+
+static netdev_tx_t l2tp_tunl_dev_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	const struct ip_tunnel_info *info;
+	struct l2tp_tunl_dev_priv *priv;
+	struct l2tp_session *session;
+	struct l2tp_tunnel *tunnel;
+	u32 tid, sid;
+
+	info = skb_tunnel_info(skb);
+	if (!info || !(info->mode & IP_TUNNEL_INFO_TX))
+		goto drop;
+
+	priv = netdev_priv(dev);
+	tid = priv->tunnel_id;
+	tunnel = l2tp_tunnel_get(dev_net(dev), tid);
+	if (!tunnel) {
+		pr_err("%s: no tunnel %u in this netns", dev->name, tid);
+		goto drop;
+	}
+
+	sid = be32_to_cpu(tunnel_id_to_key32(info->key.tun_id));
+	session = l2tp_tunnel_get_session(tunnel, sid);
+	if (!session) {
+		pr_err("%s: no session %u in tunnel %u", dev->name, sid, tid);
+		goto drop_unref_tunnel;
+	}
+
+	if (l2tp_xmit_skb(session, skb) != NET_XMIT_SUCCESS)
+		goto drop_unref_tunnel_and_session;
+
+	l2tp_session_dec_refcount(session);
+	l2tp_tunnel_dec_refcount(tunnel);
+
+	return NETDEV_TX_OK;
+
+drop_unref_tunnel_and_session:
+	l2tp_session_dec_refcount(session);
+drop_unref_tunnel:
+	l2tp_tunnel_dec_refcount(tunnel);
+drop:
+	dev_kfree_skb(skb);
+	return NETDEV_TX_OK;
+}
+
+static rx_handler_result_t l2tp_tunl_dev_rx_handler(struct sk_buff **pskb)
+{
+	struct sk_buff *skb = *pskb;
+
+	/* If a packet hasn't been redirected using tc rules we
+	 * don't want it to continue through the network stack,
+	 * so consume it now.
+	 */
+	kfree_skb(skb);
+
+	return RX_HANDLER_CONSUMED;
+}
+
+static const struct net_device_ops l2tp_tunnel_netdev_ops = {
+	.ndo_init		= l2tp_tunl_dev_init,
+	.ndo_uninit		= l2tp_tunl_dev_uninit,
+	.ndo_start_xmit		= l2tp_tunl_dev_xmit,
+};
+
+static struct device_type l2tpvt_type = {
+	.name = "l2tpvt",
+};
+
+static void l2tp_tunnel_netdev_setup(struct net_device *dev)
+{
+	SET_NETDEV_DEVTYPE(dev, &l2tpvt_type);
+	eth_hw_addr_random(dev);
+	ether_setup(dev);
+	eth_hw_addr_random(dev);
+	dev->netdev_ops = &l2tp_tunnel_netdev_ops;
+	dev->features |= NETIF_F_LLTX;
+	dev->min_mtu = ETH_MIN_MTU;
+	dev->max_mtu = ETH_MAX_MTU;
+	dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
+}
+
+static struct net_device *l2tp_tunnel_netdev_alloc(struct net *net, u32 tunnel_id)
+{
+	struct l2tp_tunl_dev_priv *priv;
+	struct net_device *dev;
+	char name[IFNAMSIZ];
+	int ret;
+
+	snprintf(name, IFNAMSIZ, "l2tpt%d", tunnel_id);
+
+	dev = alloc_netdev(sizeof(*priv), name, NET_NAME_USER, l2tp_tunnel_netdev_setup);
+	dev_net_set(dev, net);
+
+	ret = register_netdev(dev);
+	if (ret < 0) {
+		pr_err("%s: register_netdev said %d\n", __func__, ret);
+		free_netdev(dev);
+		return NULL;
+	}
+
+	/* Add rx handler to prevent packets entering the stack.
+	 * The intention here is to allow packets on the tunnel device to
+	 * be seen by sch_handle_ingress (and hence tc classifiers/actions)
+	 * but prevent them from being passed to protocol code.
+	 */
+	rtnl_lock();
+	ret = netdev_rx_handler_register(dev, l2tp_tunl_dev_rx_handler, NULL);
+	rtnl_unlock();
+	if (ret < 0) {
+		pr_err("%s: netdev_rx_handler_register said %d\n", __func__, ret);
+		free_netdev(dev);
+		return NULL;
+	}
+
+	priv = netdev_priv(dev);
+	priv->tunnel_id = tunnel_id;
+
+	return dev;
+}
+
 /*****************************************************************************
  * Init and cleanup
  *****************************************************************************/
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index 98ea98eb9567..4d2aeb852f38 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -185,6 +185,7 @@ struct l2tp_tunnel {
 						 */
 
 	struct work_struct	del_work;
+	struct net_device	*dev;
 };
 
 /* Pseudowire ops callbacks for use with the l2tp genetlink interface */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [RFC PATCH net-next 2/3] net/l2tp: add flow-based session create API
  2021-09-29  9:45 [RFC PATCH net-next 0/3] support "flow-based" datapath in l2tp Tom Parkin
  2021-09-29  9:45 ` [RFC PATCH net-next 1/3] net/l2tp: add virtual tunnel device Tom Parkin
@ 2021-09-29  9:45 ` Tom Parkin
  2021-09-29  9:45 ` [RFC PATCH net-next 3/3] net/l2tp: add netlink attribute to enable flow-based session creation Tom Parkin
  2021-09-29 10:03 ` [RFC PATCH net-next 0/3] support "flow-based" datapath in l2tp Eyal Birger
  3 siblings, 0 replies; 6+ messages in thread
From: Tom Parkin @ 2021-09-29  9:45 UTC (permalink / raw)
  To: netdev; +Cc: jchapman, Tom Parkin

In order to support multiple logical sessions over a single L2TP tunnel
virtual netdev, the L2TP session creation API needs to be extended.  The
new "flow-based" sessions will not create a virtual net device
per-session, but instead will use tc rules and tunnel metadata to direct
traffic:

        tc qdisc add dev l2tpt1 handle ffff: ingress
        tc filter add dev l2tpt1 parent ffff: flower enc_key_id 1 \
                action mirred egress redirect dev eth0

To allow this session type to co-exist with the existing pseudowire
types, define a new API for creating flow-based sessions which the
l2tp netlink code can call directly.

Signed-off-by: Tom Parkin <tparkin@katalix.com>
---
 net/l2tp/l2tp_core.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
 net/l2tp/l2tp_core.h |  8 ++++++++
 2 files changed, 54 insertions(+)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 6a4d3d785c65..dd0b1d64fd14 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1661,6 +1661,52 @@ struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunn
 }
 EXPORT_SYMBOL_GPL(l2tp_session_create);
 
+static void l2tp_flow_recv(struct l2tp_session *session, struct sk_buff *skb, int data_len)
+{
+	struct metadata_dst *mdst;
+	__be64 id;
+
+	if (!session->tunnel->dev)
+		goto drop;
+
+	id = key32_to_tunnel_id(htonl(session->session_id));
+
+	mdst = ip_tun_rx_dst(skb, TUNNEL_KEY, id, sizeof(*mdst));
+	if (!mdst)
+		goto drop;
+
+	skb->skb_iif = skb->dev->ifindex;
+	skb->dev = session->tunnel->dev;
+	skb_dst_set(skb, (struct dst_entry *)mdst);
+	skb_reset_mac_header(skb);
+	netif_receive_skb(skb);
+	return;
+
+drop:
+	kfree_skb(skb);
+}
+
+int l2tp_flow_session_create(struct l2tp_tunnel *tunnel,
+			     u32 session_id, u32 peer_session_id,
+			     struct l2tp_session_cfg *cfg)
+{
+	struct l2tp_session *session;
+	int ret;
+
+	session = l2tp_session_create(0, tunnel, session_id, peer_session_id, cfg);
+	if (IS_ERR(session)) {
+		ret = PTR_ERR(session);
+		goto out;
+	}
+
+	session->recv_skb = l2tp_flow_recv;
+
+	ret = l2tp_session_register(session, tunnel);
+out:
+	return ret;
+}
+EXPORT_SYMBOL_GPL(l2tp_flow_session_create);
+
 /*****************************************************************************
  * Tunnel virtual netdev
  *****************************************************************************/
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index 4d2aeb852f38..1a0b9859a7e1 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -252,6 +252,14 @@ int l2tp_session_register(struct l2tp_session *session,
 			  struct l2tp_tunnel *tunnel);
 void l2tp_session_delete(struct l2tp_session *session);
 
+/* Flow-based session.
+ * Optimised datapath which doesn't require a netdev per session instance
+ * and which is managed from userspace using tc rules.
+ */
+int l2tp_flow_session_create(struct l2tp_tunnel *tunnel,
+			     u32 session_id, u32 peer_session_id,
+			     struct l2tp_session_cfg *cfg);
+
 /* Receive path helpers.  If data sequencing is enabled for the session these
  * functions handle queuing and reordering prior to passing packets to the
  * pseudowire code to be passed to userspace.
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [RFC PATCH net-next 3/3] net/l2tp: add netlink attribute to enable flow-based session creation
  2021-09-29  9:45 [RFC PATCH net-next 0/3] support "flow-based" datapath in l2tp Tom Parkin
  2021-09-29  9:45 ` [RFC PATCH net-next 1/3] net/l2tp: add virtual tunnel device Tom Parkin
  2021-09-29  9:45 ` [RFC PATCH net-next 2/3] net/l2tp: add flow-based session create API Tom Parkin
@ 2021-09-29  9:45 ` Tom Parkin
  2021-09-29 10:03 ` [RFC PATCH net-next 0/3] support "flow-based" datapath in l2tp Eyal Birger
  3 siblings, 0 replies; 6+ messages in thread
From: Tom Parkin @ 2021-09-29  9:45 UTC (permalink / raw)
  To: netdev; +Cc: jchapman, Tom Parkin

Add an attribute to be used with L2TP_CMD_SESSION_CREATE in order to
enable the use of the flow-based datapath.

If the attribute is not included in the message, or is set false, the
traditional pseudowire lookup is used, which will lead to a virtual
session netdev being created.

If the attribute is included and is set, no virtual session netdev will
be created, and the administrator must use appropriate tc filter/action
rules in order to manage session data packets.

Signed-off-by: Tom Parkin <tparkin@katalix.com>
---
 include/uapi/linux/l2tp.h |  1 +
 net/l2tp/l2tp_netlink.c   | 36 +++++++++++++++++++++++-------------
 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/include/uapi/linux/l2tp.h b/include/uapi/linux/l2tp.h
index bab8c9708611..86e02c8e97a4 100644
--- a/include/uapi/linux/l2tp.h
+++ b/include/uapi/linux/l2tp.h
@@ -127,6 +127,7 @@ enum {
 	L2TP_ATTR_UDP_ZERO_CSUM6_TX,	/* flag */
 	L2TP_ATTR_UDP_ZERO_CSUM6_RX,	/* flag */
 	L2TP_ATTR_PAD,
+	L2TP_ATTR_FLOW_DATAPATH,	/* flag */
 	__L2TP_ATTR_MAX,
 };
 
diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c
index 96eb91be9238..5fb5fca74abd 100644
--- a/net/l2tp/l2tp_netlink.c
+++ b/net/l2tp/l2tp_netlink.c
@@ -528,6 +528,7 @@ static int l2tp_nl_cmd_session_create(struct sk_buff *skb, struct genl_info *inf
 	struct l2tp_session *session;
 	struct l2tp_session_cfg cfg = { 0, };
 	struct net *net = genl_info_net(info);
+	bool flow_path = false;
 
 	if (!info->attrs[L2TP_ATTR_CONN_ID]) {
 		ret = -EINVAL;
@@ -617,22 +618,30 @@ static int l2tp_nl_cmd_session_create(struct sk_buff *skb, struct genl_info *inf
 	if (info->attrs[L2TP_ATTR_RECV_TIMEOUT])
 		cfg.reorder_timeout = nla_get_msecs(info->attrs[L2TP_ATTR_RECV_TIMEOUT]);
 
+	if (info->attrs[L2TP_ATTR_FLOW_DATAPATH])
+		flow_path = nla_get_flag(info->attrs[L2TP_ATTR_FLOW_DATAPATH]);
+
+	if (flow_path) {
+		ret = l2tp_flow_session_create(tunnel, session_id, peer_session_id, &cfg);
+	} else {
 #ifdef CONFIG_MODULES
-	if (!l2tp_nl_cmd_ops[cfg.pw_type]) {
-		genl_unlock();
-		request_module("net-l2tp-type-%u", cfg.pw_type);
-		genl_lock();
-	}
+		if (!l2tp_nl_cmd_ops[cfg.pw_type]) {
+			genl_unlock();
+			request_module("net-l2tp-type-%u", cfg.pw_type);
+			genl_lock();
+		}
 #endif
-	if (!l2tp_nl_cmd_ops[cfg.pw_type] || !l2tp_nl_cmd_ops[cfg.pw_type]->session_create) {
-		ret = -EPROTONOSUPPORT;
-		goto out_tunnel;
-	}
+		if (!l2tp_nl_cmd_ops[cfg.pw_type] ||
+		    !l2tp_nl_cmd_ops[cfg.pw_type]->session_create) {
+			ret = -EPROTONOSUPPORT;
+			goto out_tunnel;
+		}
 
-	ret = l2tp_nl_cmd_ops[cfg.pw_type]->session_create(net, tunnel,
-							   session_id,
-							   peer_session_id,
-							   &cfg);
+		ret = l2tp_nl_cmd_ops[cfg.pw_type]->session_create(net, tunnel,
+								   session_id,
+								   peer_session_id,
+								   &cfg);
+	}
 
 	if (ret >= 0) {
 		session = l2tp_tunnel_get_session(tunnel, session_id);
@@ -918,6 +927,7 @@ static const struct nla_policy l2tp_nl_policy[L2TP_ATTR_MAX + 1] = {
 		.type = NLA_BINARY,
 		.len = 8,
 	},
+	[L2TP_ATTR_FLOW_DATAPATH]	= { .type = NLA_U8 },
 };
 
 static const struct genl_small_ops l2tp_nl_ops[] = {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH net-next 0/3] support "flow-based" datapath in l2tp
  2021-09-29  9:45 [RFC PATCH net-next 0/3] support "flow-based" datapath in l2tp Tom Parkin
                   ` (2 preceding siblings ...)
  2021-09-29  9:45 ` [RFC PATCH net-next 3/3] net/l2tp: add netlink attribute to enable flow-based session creation Tom Parkin
@ 2021-09-29 10:03 ` Eyal Birger
  2021-10-01  8:40   ` Tom Parkin
  3 siblings, 1 reply; 6+ messages in thread
From: Eyal Birger @ 2021-09-29 10:03 UTC (permalink / raw)
  To: Tom Parkin; +Cc: jchapman, netdev

Hi Tom,

On 29/09/2021 12:45, Tom Parkin wrote:
...
>        The skb is then redirected to the tunnel virtual netdev: tc rules
>        can then be added to match traffic based on the session ID and
>        redirect it to the correct interface:
> 
>              tc qdisc add dev l2tpt1 handle ffff: ingress
>              tc filter add dev l2tpt1 \
>                      parent ffff: \
>                      flower enc_key_id 1 \
>                      action mirred egress redirect dev eth0
> 
>        In the case that no tc rule matches an incoming packet, the tunnel
>        virtual device implements an rx handler which swallows the packet
>        in order to prevent it continuing through the network stack.

There are other ways to utilize the tunnel key on rx, e.g. in ip rules.

IMHO it'd be nicer if the decision to drop would be an administrator 
decision which they can implement using a designated tc drop rule.

Eyal.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH net-next 0/3] support "flow-based" datapath in l2tp
  2021-09-29 10:03 ` [RFC PATCH net-next 0/3] support "flow-based" datapath in l2tp Eyal Birger
@ 2021-10-01  8:40   ` Tom Parkin
  0 siblings, 0 replies; 6+ messages in thread
From: Tom Parkin @ 2021-10-01  8:40 UTC (permalink / raw)
  To: Eyal Birger; +Cc: jchapman, netdev

[-- Attachment #1: Type: text/plain, Size: 1257 bytes --]

On  Wed, Sep 29, 2021 at 13:03:21 +0300, Eyal Birger wrote:
> Hi Tom,
> 
> On 29/09/2021 12:45, Tom Parkin wrote:
> ...
> >        The skb is then redirected to the tunnel virtual netdev: tc rules
> >        can then be added to match traffic based on the session ID and
> >        redirect it to the correct interface:
> > 
> >              tc qdisc add dev l2tpt1 handle ffff: ingress
> >              tc filter add dev l2tpt1 \
> >                      parent ffff: \
> >                      flower enc_key_id 1 \
> >                      action mirred egress redirect dev eth0
> > 
> >        In the case that no tc rule matches an incoming packet, the tunnel
> >        virtual device implements an rx handler which swallows the packet
> >        in order to prevent it continuing through the network stack.
> 
> There are other ways to utilize the tunnel key on rx, e.g. in ip rules.
> 
> IMHO it'd be nicer if the decision to drop would be an administrator
> decision which they can implement using a designated tc drop rule.

Good point, and one I hadn't considered.

My concern with letting the packet enter the stack is that it could
possibly cause issues, but maybe it's better to allow the admin to
make that call.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-10-01  8:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-29  9:45 [RFC PATCH net-next 0/3] support "flow-based" datapath in l2tp Tom Parkin
2021-09-29  9:45 ` [RFC PATCH net-next 1/3] net/l2tp: add virtual tunnel device Tom Parkin
2021-09-29  9:45 ` [RFC PATCH net-next 2/3] net/l2tp: add flow-based session create API Tom Parkin
2021-09-29  9:45 ` [RFC PATCH net-next 3/3] net/l2tp: add netlink attribute to enable flow-based session creation Tom Parkin
2021-09-29 10:03 ` [RFC PATCH net-next 0/3] support "flow-based" datapath in l2tp Eyal Birger
2021-10-01  8:40   ` Tom Parkin

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.