netdev - RE: [RFC PATCH net-next 1/4] flow_dissector: Add PPPoE dissectors

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:   Sun, 26 Jun 2022 07:20:18 +0000
From:   Baowen Zheng <baowen.zheng@...igine.com>
To:     Marcin Szycik <marcin.szycik@...ux.intel.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:     "anthony.l.nguyen@...el.com" <anthony.l.nguyen@...el.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "xiyou.wangcong@...il.com" <xiyou.wangcong@...il.com>,
        "jesse.brandeburg@...el.com" <jesse.brandeburg@...el.com>,
        "gustavoars@...nel.org" <gustavoars@...nel.org>,
        "boris.sukholitko@...adcom.com" <boris.sukholitko@...adcom.com>,
        "elic@...dia.com" <elic@...dia.com>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "jhs@...atatu.com" <jhs@...atatu.com>,
        "jiri@...nulli.us" <jiri@...nulli.us>,
        "kurt@...utronix.de" <kurt@...utronix.de>,
        "pablo@...filter.org" <pablo@...filter.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "paulb@...dia.com" <paulb@...dia.com>,
        Simon Horman <simon.horman@...igine.com>,
        "komachi.yoshiki@...il.com" <komachi.yoshiki@...il.com>,
        "zhangkaiheb@....com" <zhangkaiheb@....com>,
        "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
        "michal.swiatkowski@...ux.intel.com" 
        <michal.swiatkowski@...ux.intel.com>,
        "wojciech.drewek@...el.com" <wojciech.drewek@...el.com>
Subject: RE: [RFC PATCH net-next 1/4] flow_dissector: Add PPPoE dissectors

On Friday, June 24, 2022 9:42 PM, Marcin Szycik wrote:

>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@...el.com>
>---
> include/net/flow_dissector.h | 11 ++++++++
> net/core/flow_dissector.c    | 51 +++++++++++++++++++++++++++++++-----
> 2 files changed, 56 insertions(+), 6 deletions(-)
>
>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;
> };
>
[..]
>+static bool is_ppp_proto_supported(__be16 proto) {
>+	switch (proto) {
>+	case htons(PPP_AT):
>+	case htons(PPP_IPX):
>+	case htons(PPP_VJC_COMP):
>+	case htons(PPP_VJC_UNCOMP):
>+	case htons(PPP_MP):
>+	case htons(PPP_COMPFRAG):
>+	case htons(PPP_COMP):
>+	case htons(PPP_MPLS_UC):
>+	case htons(PPP_MPLS_MC):
>+	case htons(PPP_IPCP):
>+	case htons(PPP_ATCP):
>+	case htons(PPP_IPXCP):
>+	case htons(PPP_IPV6CP):
>+	case htons(PPP_CCPFRAG):
>+	case htons(PPP_MPLSCP):
>+	case htons(PPP_LCP):
>+	case htons(PPP_PAP):
>+	case htons(PPP_LQR):
>+	case htons(PPP_CHAP):
>+	case htons(PPP_CBCP):
>+		return true;
>+	default:
>+		return false;
>+	}
>+}
>+
Just a suggestion, for the above code segment, will it be more clean to change as the format:
	switch (ntohs(proto)) {
	case PPP_AT:
	case PPP_IPX:
	case PPP_VJC_COMP:
then you will not need to call function of htons repeatedly.
> /**
>  * __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): {
>--
>2.35.1