diff mbox series

[RFC,net-next,v2,1/4] flow_dissector: Add PPPoE dissectors

Message ID 20220628112918.11296-2-marcin.szycik@linux.intel.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series ice: PPPoE offload support | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 5647 this patch: 5647
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 1211 this patch: 1211
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 5796 this patch: 5796
netdev/checkpatch warning WARNING: line length of 87 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 7 this patch: 7
netdev/source_inline success Was 0 now: 0

Commit Message

Marcin Szycik June 28, 2022, 11:29 a.m. UTC 
From: Wojciech Drewek <wojciech.drewek@intel.com>

Allow to dissect PPPoE specific fields which are:
- session ID (16 bits)
- ppp protocol (16 bits)

The goal is to make the following TC command possible:

  # tc filter add dev ens6f0 ingress prio 1 protocol ppp_ses \
      flower \
        pppoe_sid 12 \
        ppp_proto ip \
      action drop

Note that only PPPoE Session is supported.

Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
---
v2: use ntohs instead of htons in is_ppp_proto_supported

 include/net/flow_dissector.h | 11 ++++++++
 net/core/flow_dissector.c    | 51 +++++++++++++++++++++++++++++++-----
 2 files changed, 56 insertions(+), 6 deletions(-)

Comments

Jakub Kicinski June 29, 2022, 4:40 a.m. UTC | #1 
On Tue, 28 Jun 2022 13:29:15 +0200 Marcin Szycik wrote:
> +static bool is_ppp_proto_supported(__be16 proto)

What does supported mean in this context?

> +{
> +	switch (ntohs(proto)) {
> +	case PPP_AT:

Byte swap on the constant.

> +	case PPP_IPX:
> +	case PPP_VJC_COMP:
> +	case PPP_VJC_UNCOMP:
> +	case PPP_MP:
> +	case PPP_COMPFRAG:
> +	case PPP_COMP:
> +	case PPP_MPLS_UC:
> +	case PPP_MPLS_MC:
> +	case PPP_IPCP:
> +	case PPP_ATCP:
> +	case PPP_IPXCP:
> +	case PPP_IPV6CP:
> +	case PPP_CCPFRAG:
> +	case PPP_MPLSCP:
> +	case PPP_LCP:
> +	case PPP_PAP:
> +	case PPP_LQR:
> +	case PPP_CHAP:
> +	case PPP_CBCP:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
Wojciech Drewek June 29, 2022, 7:44 a.m. UTC | #2 
> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: środa, 29 czerwca 2022 06:40
> To: Marcin Szycik <marcin.szycik@linux.intel.com>
> Cc: netdev@vger.kernel.org; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; davem@davemloft.net;
> xiyou.wangcong@gmail.com; Brandeburg, Jesse <jesse.brandeburg@intel.com>; gustavoars@kernel.org;
> baowen.zheng@corigine.com; boris.sukholitko@broadcom.com; edumazet@google.com; jhs@mojatatu.com; jiri@resnulli.us;
> kurt@linutronix.de; pablo@netfilter.org; pabeni@redhat.com; paulb@nvidia.com; simon.horman@corigine.com;
> komachi.yoshiki@gmail.com; zhangkaiheb@126.com; intel-wired-lan@lists.osuosl.org; michal.swiatkowski@linux.intel.com; Drewek,
> Wojciech <wojciech.drewek@intel.com>; Lobakin, Alexandr <alexandr.lobakin@intel.com>
> Subject: Re: [RFC PATCH net-next v2 1/4] flow_dissector: Add PPPoE dissectors
> 
> On Tue, 28 Jun 2022 13:29:15 +0200 Marcin Szycik wrote:
> > +static bool is_ppp_proto_supported(__be16 proto)
> 
> What does supported mean in this context?

It means that only those protocols are going to be dissected.

> 
> > +{
> > +	switch (ntohs(proto)) {
> > +	case PPP_AT:
> 
> Byte swap on the constant.

Sure, it will be included in v3

> 
> > +	case PPP_IPX:
> > +	case PPP_VJC_COMP:
> > +	case PPP_VJC_UNCOMP:
> > +	case PPP_MP:
> > +	case PPP_COMPFRAG:
> > +	case PPP_COMP:
> > +	case PPP_MPLS_UC:
> > +	case PPP_MPLS_MC:
> > +	case PPP_IPCP:
> > +	case PPP_ATCP:
> > +	case PPP_IPXCP:
> > +	case PPP_IPV6CP:
> > +	case PPP_CCPFRAG:
> > +	case PPP_MPLSCP:
> > +	case PPP_LCP:
> > +	case PPP_PAP:
> > +	case PPP_LQR:
> > +	case PPP_CHAP:
> > +	case PPP_CBCP:
> > +		return true;
> > +	default:
> > +		return false;
> > +	}
> > +}
Jakub Kicinski June 29, 2022, 3:19 p.m. UTC | #3 
On Wed, 29 Jun 2022 07:44:50 +0000 Drewek, Wojciech wrote:
> > > +static bool is_ppp_proto_supported(__be16 proto)  
> > 
> > What does supported mean in this context?  
> 
> It means that only those protocols are going to be dissected.

We only dissect PPP_IP and PPP_IPV6. This looks more like a list of all
known protocols.
Wojciech Drewek July 1, 2022, 10:58 a.m. UTC | #4 
> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: środa, 29 czerwca 2022 17:20
> To: Drewek, Wojciech <wojciech.drewek@intel.com>
> Cc: Marcin Szycik <marcin.szycik@linux.intel.com>; netdev@vger.kernel.org; Nguyen, Anthony L <anthony.l.nguyen@intel.com>;
> davem@davemloft.net; xiyou.wangcong@gmail.com; Brandeburg, Jesse <jesse.brandeburg@intel.com>; gustavoars@kernel.org;
> baowen.zheng@corigine.com; boris.sukholitko@broadcom.com; edumazet@google.com; jhs@mojatatu.com; jiri@resnulli.us;
> kurt@linutronix.de; pablo@netfilter.org; pabeni@redhat.com; paulb@nvidia.com; simon.horman@corigine.com;
> komachi.yoshiki@gmail.com; zhangkaiheb@126.com; intel-wired-lan@lists.osuosl.org; michal.swiatkowski@linux.intel.com; Lobakin,
> Alexandr <alexandr.lobakin@intel.com>
> Subject: Re: [RFC PATCH net-next v2 1/4] flow_dissector: Add PPPoE dissectors
> 
> On Wed, 29 Jun 2022 07:44:50 +0000 Drewek, Wojciech wrote:
> > > > +static bool is_ppp_proto_supported(__be16 proto)
> > >
> > > What does supported mean in this context?
> >
> > It means that only those protocols are going to be dissected.
> 
> We only dissect PPP_IP and PPP_IPV6. This looks more like a list of all
> known protocols.

Sorry, maybe I wasn't precise enough. It is a list of protocols that user can match on.
In case of PPP_IP and PPP_IPV6 user can also match on inner fields specific for ipv4
and ipv6.
diff mbox series

Patch

diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
index a4c6057c7097..8ff40c7c3f1c 100644
--- a/include/net/flow_dissector.h
+++ b/include/net/flow_dissector.h
@@ -261,6 +261,16 @@  struct flow_dissector_key_num_of_vlans {
 	u8 num_of_vlans;
 };
 
+/**
+ * struct flow_dissector_key_pppoe:
+ * @session_id: pppoe session id
+ * @ppp_proto: ppp protocol
+ */
+struct flow_dissector_key_pppoe {
+	u16 session_id;
+	__be16 ppp_proto;
+};
+
 enum flow_dissector_key_id {
 	FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */
 	FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */
@@ -291,6 +301,7 @@  enum flow_dissector_key_id {
 	FLOW_DISSECTOR_KEY_CT, /* struct flow_dissector_key_ct */
 	FLOW_DISSECTOR_KEY_HASH, /* struct flow_dissector_key_hash */
 	FLOW_DISSECTOR_KEY_NUM_OF_VLANS, /* struct flow_dissector_key_num_of_vlans */
+	FLOW_DISSECTOR_KEY_PPPOE,  /* struct flow_dissector_key_pppoe */
 
 	FLOW_DISSECTOR_KEY_MAX,
 };
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 6aee04f75e3e..a758b1980e4a 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -895,6 +895,35 @@  bool bpf_flow_dissect(struct bpf_prog *prog, struct bpf_flow_dissector *ctx,
 	return result == BPF_OK;
 }
 
+static bool is_ppp_proto_supported(__be16 proto)
+{
+	switch (ntohs(proto)) {
+	case PPP_AT:
+	case PPP_IPX:
+	case PPP_VJC_COMP:
+	case PPP_VJC_UNCOMP:
+	case PPP_MP:
+	case PPP_COMPFRAG:
+	case PPP_COMP:
+	case PPP_MPLS_UC:
+	case PPP_MPLS_MC:
+	case PPP_IPCP:
+	case PPP_ATCP:
+	case PPP_IPXCP:
+	case PPP_IPV6CP:
+	case PPP_CCPFRAG:
+	case PPP_MPLSCP:
+	case PPP_LCP:
+	case PPP_PAP:
+	case PPP_LQR:
+	case PPP_CHAP:
+	case PPP_CBCP:
+		return true;
+	default:
+		return false;
+	}
+}
+
 /**
  * __skb_flow_dissect - extract the flow_keys struct and return it
  * @net: associated network namespace, derived from @skb if NULL
@@ -1221,19 +1250,29 @@  bool __skb_flow_dissect(const struct net *net,
 		}
 
 		nhoff += PPPOE_SES_HLEN;
-		switch (hdr->proto) {
-		case htons(PPP_IP):
+		if (hdr->proto == htons(PPP_IP)) {
 			proto = htons(ETH_P_IP);
 			fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
-			break;
-		case htons(PPP_IPV6):
+		} else if (hdr->proto == htons(PPP_IPV6)) {
 			proto = htons(ETH_P_IPV6);
 			fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
-			break;
-		default:
+		} else if (is_ppp_proto_supported(hdr->proto)) {
+			fdret = FLOW_DISSECT_RET_OUT_GOOD;
+		} else {
 			fdret = FLOW_DISSECT_RET_OUT_BAD;
 			break;
 		}
+
+		if (dissector_uses_key(flow_dissector,
+				       FLOW_DISSECTOR_KEY_PPPOE)) {
+			struct flow_dissector_key_pppoe *key_pppoe;
+
+			key_pppoe = skb_flow_dissector_target(flow_dissector,
+							      FLOW_DISSECTOR_KEY_PPPOE,
+							      target_container);
+			key_pppoe->session_id = ntohs(hdr->hdr.sid);
+			key_pppoe->ppp_proto = hdr->proto;
+		}
 		break;
 	}
 	case htons(ETH_P_TIPC): {