[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <MW4PR11MB57765C5D5A1149515E3C6F97FDB99@MW4PR11MB5776.namprd11.prod.outlook.com>
Date: Mon, 27 Jun 2022 11:36:21 +0000
From: "Drewek, Wojciech" <wojciech.drewek@...el.com>
To: Baowen Zheng <baowen.zheng@...igine.com>,
Marcin Szycik <marcin.szycik@...ux.intel.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC: "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>,
"davem@...emloft.net" <davem@...emloft.net>,
"xiyou.wangcong@...il.com" <xiyou.wangcong@...il.com>,
"Brandeburg, Jesse" <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>
Subject: RE: [RFC PATCH net-next 1/4] flow_dissector: Add PPPoE dissectors
Thanks for the review!
> -----Original Message-----
> From: Baowen Zheng <baowen.zheng@...igine.com>
> Sent: niedziela, 26 czerwca 2022 09:20
> To: Marcin Szycik <marcin.szycik@...ux.intel.com>; netdev@...r.kernel.org
> Cc: Nguyen, Anthony L <anthony.l.nguyen@...el.com>; davem@...emloft.net; xiyou.wangcong@...il.com; Brandeburg, Jesse
> <jesse.brandeburg@...el.com>; gustavoars@...nel.org; boris.sukholitko@...adcom.com; elic@...dia.com; edumazet@...gle.com;
> kuba@...nel.org; jhs@...atatu.com; jiri@...nulli.us; kurt@...utronix.de; pablo@...filter.org; pabeni@...hat.com;
> paulb@...dia.com; Simon Horman <simon.horman@...igine.com>; komachi.yoshiki@...il.com; zhangkaiheb@....com; intel-wired-
> lan@...ts.osuosl.org; michal.swiatkowski@...ux.intel.com; Drewek, Wojciech <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.
Sure, will be included in second version.
> > /**
> > * __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
Powered by blists - more mailing lists