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] [day] [month] [year] [list]
Date:   Mon, 27 Jun 2022 11:33:23 +0000
From:   "Drewek, Wojciech" <wojciech.drewek@...el.com>
To:     "Lobakin, Alexandr" <alexandr.lobakin@...el.com>,
        Marcin Szycik <marcin.szycik@...ux.intel.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "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>,
        "baowen.zheng@...igine.com" <baowen.zheng@...igine.com>,
        "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@...igine.com" <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



> -----Original Message-----
> From: Lobakin, Alexandr <alexandr.lobakin@...el.com>
> Sent: piÄ…tek, 24 czerwca 2022 17:01
> To: Marcin Szycik <marcin.szycik@...ux.intel.com>
> Cc: Lobakin, Alexandr <alexandr.lobakin@...el.com>; netdev@...r.kernel.org; Nguyen, Anthony L <anthony.l.nguyen@...el.com>;
> davem@...emloft.net; xiyou.wangcong@...il.com; Brandeburg, Jesse <jesse.brandeburg@...el.com>; gustavoars@...nel.org;
> baowen.zheng@...igine.com; 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@...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
> 
> From: Marcin Szycik <marcin.szycik@...ux.intel.com>
> Date: Fri, 24 Jun 2022 15:41:31 +0200
> 
> > From: Wojciech Drewek <wojciech.drewek@...el.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@...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;
> >  };
> >
> > +/**
> > + * 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..41933905f90b 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 (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;
> > +	}
> > +}
> > +
> >  /**
> >   * __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:
> 
> Oh, sorry for missing this previously. This switch -> if-else
> conversion looks redundant.
> You can add `if (is_ppp_proto_supported()) GOOD; else BAD` to the
> `default` label without modifying the rest of code (to skip the
> actual dissecting block you can add a condition there that @fdret
> must not equal BAD).

I could leave switch case statement but IMHO converting to
if else statement is cleaner then adding new if after switch case
in order to skip dissecting block.

> 
> > +		} 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
> 
> Thanks,
> Olek

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ