[PATCH iproute-next v2 0/3] PPPoE support in tc-flower

[PATCH iproute-next v2 0/3] PPPoE support in tc-flower

[Wojciech Drewek]

This patchset implements support for matching
on PPPoE specific fields using tc-flower.
First patch introduces small refactor which allows
to use same mechanism of finding protocol for
both ppp and ether protocols. Second patch
adds support for parsing ppp protocols.
Last patch is about parsing PPPoE fields.

Kernel changes:
https://lore.kernel.org/netdev/20220726203133.2171332-1-anthony.l.nguyen@intel.com/T/#t

Wojciech Drewek (3):
  lib: refactor ll_proto functions
  lib: Introduce ppp protocols
  f_flower: Introduce PPPoE support

 include/rt_names.h            |  3 ++
 include/uapi/linux/pkt_cls.h  |  3 ++
 include/uapi/linux/ppp_defs.h | 37 ++++++++++++++++++++++
 include/utils.h               | 10 ++++++
 lib/Makefile                  |  2 +-
 lib/ll_proto.c                | 33 +++++---------------
 lib/ppp_proto.c               | 52 +++++++++++++++++++++++++++++++
 lib/utils.c                   | 34 ++++++++++++++++++++
 man/man8/tc-flower.8          | 17 +++++++++-
 tc/f_flower.c                 | 58 +++++++++++++++++++++++++++++++++++
 10 files changed, 222 insertions(+), 27 deletions(-)
 create mode 100644 include/uapi/linux/ppp_defs.h
 create mode 100644 lib/ppp_proto.c

-- 
2.31.1

[PATCH iproute-next v2 1/3] lib: refactor ll_proto functions

[Wojciech Drewek]

Move core logic of ll_proto_n2a and ll_proto_a2n
to utils.c and make it more generic by allowing to
pass table of protocols as argument (proto_tb).
Introduce struct proto with protocol ID and name to
allow this. This wil allow to use those functions by
other use cases.

Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
---
 include/utils.h | 10 ++++++++++
 lib/ll_proto.c  | 33 ++++++++-------------------------
 lib/utils.c     | 34 ++++++++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 25 deletions(-)

diff --git a/include/utils.h b/include/utils.h
index 9765fdd231df..eeb23a64f008 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -369,4 +369,14 @@ void inc_indent(struct indent_mem *mem);
 void dec_indent(struct indent_mem *mem);
 void print_indent(struct indent_mem *mem);
 
+struct proto {
+	int id;
+	const char *name;
+};
+
+int proto_a2n(unsigned short *id, const char *buf,
+	      const struct proto *proto_tb, size_t tb_len);
+const char *proto_n2a(unsigned short id, char *buf, int len,
+		      const struct proto *proto_tb, size_t tb_len);
+
 #endif /* __UTILS_H__ */
diff --git a/lib/ll_proto.c b/lib/ll_proto.c
index 342ea2eefa4c..925e2caa05e5 100644
--- a/lib/ll_proto.c
+++ b/lib/ll_proto.c
@@ -28,10 +28,8 @@
 
 
 #define __PF(f,n) { ETH_P_##f, #n },
-static const struct {
-	int id;
-	const char *name;
-} llproto_names[] = {
+
+static const struct proto llproto_names[] = {
 __PF(LOOP,loop)
 __PF(PUP,pup)
 __PF(PUPAT,pupat)
@@ -90,31 +88,16 @@ __PF(TEB,teb)
 };
 #undef __PF
 
-
-const char * ll_proto_n2a(unsigned short id, char *buf, int len)
+const char *ll_proto_n2a(unsigned short id, char *buf, int len)
 {
-        int i;
+	size_t len_tb = ARRAY_SIZE(llproto_names);
 
-	id = ntohs(id);
-
-        for (i=0; !numeric && i<sizeof(llproto_names)/sizeof(llproto_names[0]); i++) {
-                 if (llproto_names[i].id == id)
-			return llproto_names[i].name;
-	}
-        snprintf(buf, len, "[%d]", id);
-        return buf;
+	return proto_n2a(id, buf, len, llproto_names, len_tb);
 }
 
 int ll_proto_a2n(unsigned short *id, const char *buf)
 {
-        int i;
-        for (i=0; i < sizeof(llproto_names)/sizeof(llproto_names[0]); i++) {
-                 if (strcasecmp(llproto_names[i].name, buf) == 0) {
-			 *id = htons(llproto_names[i].id);
-			 return 0;
-		 }
-	}
-	if (get_be16(id, buf, 0))
-		return -1;
-	return 0;
+	size_t len_tb = ARRAY_SIZE(llproto_names);
+
+	return proto_a2n(id, buf, llproto_names, len_tb);
 }
diff --git a/lib/utils.c b/lib/utils.c
index 53d310060284..dd3cdb31239c 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1925,3 +1925,37 @@ void print_indent(struct indent_mem *mem)
 	if (mem->indent_level)
 		printf("%s", mem->indent_str);
 }
+
+const char *proto_n2a(unsigned short id, char *buf, int len,
+		      const struct proto *proto_tb, size_t tb_len)
+{
+	int i;
+
+	id = ntohs(id);
+
+	for (i = 0; !numeric && i < tb_len; i++) {
+		if (proto_tb[i].id == id)
+			return proto_tb[i].name;
+	}
+
+	snprintf(buf, len, "[%d]", id);
+
+	return buf;
+}
+
+int proto_a2n(unsigned short *id, const char *buf,
+	      const struct proto *proto_tb, size_t tb_len)
+{
+	int i;
+
+	for (i = 0; i < tb_len; i++) {
+		if (strcasecmp(proto_tb[i].name, buf) == 0) {
+			*id = htons(proto_tb[i].id);
+			return 0;
+		}
+	}
+	if (get_be16(id, buf, 0))
+		return -1;
+
+	return 0;
+}
-- 
2.31.1

[PATCH iproute-next v2 2/3] lib: Introduce ppp protocols

[Wojciech Drewek]

PPP protocol field uses different values than ethertype. Introduce
utilities for translating PPP protocols from strings to values
and vice versa. Use generic API from utils in order to get
proto id and name.

Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
---
v2: Helper functions moved to separate commit
---
 include/rt_names.h            |  3 ++
 include/uapi/linux/ppp_defs.h | 37 +++++++++++++++++++++++++
 lib/Makefile                  |  2 +-
 lib/ppp_proto.c               | 52 +++++++++++++++++++++++++++++++++++
 4 files changed, 93 insertions(+), 1 deletion(-)
 create mode 100644 include/uapi/linux/ppp_defs.h
 create mode 100644 lib/ppp_proto.c

diff --git a/include/rt_names.h b/include/rt_names.h
index 1835f3be2bed..6358650db404 100644
--- a/include/rt_names.h
+++ b/include/rt_names.h
@@ -31,6 +31,9 @@ int ll_addr_a2n(char *lladdr, int len, const char *arg);
 const char * ll_proto_n2a(unsigned short id, char *buf, int len);
 int ll_proto_a2n(unsigned short *id, const char *buf);
 
+const char *ppp_proto_n2a(unsigned short id, char *buf, int len);
+int ppp_proto_a2n(unsigned short *id, const char *buf);
+
 const char *nl_proto_n2a(int id, char *buf, int len);
 int nl_proto_a2n(__u32 *id, const char *arg);
 
diff --git a/include/uapi/linux/ppp_defs.h b/include/uapi/linux/ppp_defs.h
new file mode 100644
index 000000000000..0013dc77e3b9
--- /dev/null
+++ b/include/uapi/linux/ppp_defs.h
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * ppp_defs.h - PPP definitions.
+ *
+ * Copyright 1994-2000 Paul Mackerras.
+ *
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU General Public License
+ *  version 2 as published by the Free Software Foundation.
+ */
+
+/*
+ * Protocol field values.
+ */
+#define PPP_IP		0x21	/* Internet Protocol */
+#define PPP_AT		0x29	/* AppleTalk Protocol */
+#define PPP_IPX		0x2b	/* IPX protocol */
+#define	PPP_VJC_COMP	0x2d	/* VJ compressed TCP */
+#define	PPP_VJC_UNCOMP	0x2f	/* VJ uncompressed TCP */
+#define PPP_MP		0x3d	/* Multilink protocol */
+#define PPP_IPV6	0x57	/* Internet Protocol Version 6 */
+#define PPP_COMPFRAG	0xfb	/* fragment compressed below bundle */
+#define PPP_COMP	0xfd	/* compressed packet */
+#define PPP_MPLS_UC	0x0281	/* Multi Protocol Label Switching - Unicast */
+#define PPP_MPLS_MC	0x0283	/* Multi Protocol Label Switching - Multicast */
+#define PPP_IPCP	0x8021	/* IP Control Protocol */
+#define PPP_ATCP	0x8029	/* AppleTalk Control Protocol */
+#define PPP_IPXCP	0x802b	/* IPX Control Protocol */
+#define PPP_IPV6CP	0x8057	/* IPv6 Control Protocol */
+#define PPP_CCPFRAG	0x80fb	/* CCP at link level (below MP bundle) */
+#define PPP_CCP		0x80fd	/* Compression Control Protocol */
+#define PPP_MPLSCP	0x80fd	/* MPLS Control Protocol */
+#define PPP_LCP		0xc021	/* Link Control Protocol */
+#define PPP_PAP		0xc023	/* Password Authentication Protocol */
+#define PPP_LQR		0xc025	/* Link Quality Report protocol */
+#define PPP_CHAP	0xc223	/* Cryptographic Handshake Auth. Protocol */
+#define PPP_CBCP	0xc029	/* Callback Control Protocol */
diff --git a/lib/Makefile b/lib/Makefile
index 6c98f9a61fdb..ddedd37feb32 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -5,7 +5,7 @@ CFLAGS += -fPIC
 
 UTILOBJ = utils.o utils_math.o rt_names.o ll_map.o ll_types.o ll_proto.o ll_addr.o \
 	inet_proto.o namespace.o json_writer.o json_print.o json_print_math.o \
-	names.o color.o bpf_legacy.o bpf_glue.o exec.o fs.o cg_map.o
+	names.o color.o bpf_legacy.o bpf_glue.o exec.o fs.o cg_map.o ppp_proto.o
 
 ifeq ($(HAVE_ELF),y)
 ifeq ($(HAVE_LIBBPF),y)
diff --git a/lib/ppp_proto.c b/lib/ppp_proto.c
new file mode 100644
index 000000000000..a63466432888
--- /dev/null
+++ b/lib/ppp_proto.c
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Utilities for translating PPP protocols from strings
+ * and vice versa.
+ *
+ * Authors:     Wojciech Drewek <wojciech.drewek@intel.com>
+ */
+
+#include <linux/ppp_defs.h>
+#include <linux/if_ether.h>
+#include "utils.h"
+#include "rt_names.h"
+
+static const struct proto ppp_proto_names[] = {
+	{PPP_IP, "ip"},
+	{PPP_AT, "at"},
+	{PPP_IPX, "ipx"},
+	{PPP_VJC_COMP, "vjc_comp"},
+	{PPP_VJC_UNCOMP, "vjc_uncomp"},
+	{PPP_MP, "mp"},
+	{PPP_IPV6, "ipv6"},
+	{PPP_COMPFRAG, "compfrag"},
+	{PPP_COMP, "comp"},
+	{PPP_MPLS_UC, "mpls_uc"},
+	{PPP_MPLS_MC, "mpls_mc"},
+	{PPP_IPCP, "ipcp"},
+	{PPP_ATCP, "atcp"},
+	{PPP_IPXCP, "ipxcp"},
+	{PPP_IPV6CP, "ipv6cp"},
+	{PPP_CCPFRAG, "ccpfrag"},
+	{PPP_CCP, "ccp"},
+	{PPP_MPLSCP, "mplscp"},
+	{PPP_LCP, "lcp"},
+	{PPP_PAP, "pap"},
+	{PPP_LQR, "lqr"},
+	{PPP_CHAP, "chap"},
+	{PPP_CBCP, "cbcp"},
+};
+
+const char *ppp_proto_n2a(unsigned short id, char *buf, int len)
+{
+	size_t len_tb = ARRAY_SIZE(ppp_proto_names);
+
+	return proto_n2a(id, buf, len, ppp_proto_names, len_tb);
+}
+
+int ppp_proto_a2n(unsigned short *id, const char *buf)
+{
+	size_t len_tb = ARRAY_SIZE(ppp_proto_names);
+
+	return proto_a2n(id, buf, ppp_proto_names, len_tb);
+}
-- 
2.31.1

[PATCH iproute-next v2 3/3] f_flower: Introduce PPPoE support

[Wojciech Drewek]

Introduce PPPoE specific fields in tc-flower:
- session id (16 bits)
- ppp protocol (16 bits)
Those fields can be provided only when protocol was set to
ETH_P_PPP_SES. ppp_proto works similar to vlan_ethtype, i.e.
ppp_proto overwrites eth_type. Thanks to that, fields from
encapsulated protocols (such as src_ip) can be specified.

e.g.
  # tc filter add dev ens6f0 ingress prio 1 protocol ppp_ses \
      flower \
        pppoe_sid 1234 \
        ppp_proto ip \
        dst_ip 127.0.0.1 \
        src_ip 127.0.0.2 \
      action drop

Vlan and cvlan is also supported, in this case cvlan_ethtype
or vlan_ethtype has to be set to ETH_P_PPP_SES.

e.g.
  # tc filter add dev ens6f0 ingress prio 1 protocol 802.1Q \
      flower \
        vlan_id 2 \
        vlan_ethtype ppp_ses \
        pppoe_sid 1234 \
        ppp_proto ip \
        dst_ip 127.0.0.1 \
        src_ip 127.0.0.2 \
      action drop

Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
---
v2: add pppoe fields to explain
---
 include/uapi/linux/pkt_cls.h |  3 ++
 man/man8/tc-flower.8         | 17 ++++++++++-
 tc/f_flower.c                | 58 ++++++++++++++++++++++++++++++++++++
 3 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index 9a2ee1e39fad..a67dcd8294c9 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -589,6 +589,9 @@ enum {
 
 	TCA_FLOWER_KEY_NUM_OF_VLANS,    /* u8 */
 
+	TCA_FLOWER_KEY_PPPOE_SID,	/* u16 */
+	TCA_FLOWER_KEY_PPP_PROTO,	/* be16 */
+
 	__TCA_FLOWER_MAX,
 };
 
diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
index 523935242ccf..5e486ea31d37 100644
--- a/man/man8/tc-flower.8
+++ b/man/man8/tc-flower.8
@@ -40,6 +40,10 @@ flower \- flow based traffic control filter
 .IR PRIORITY " | "
 .BR cvlan_ethtype " { " ipv4 " | " ipv6 " | "
 .IR ETH_TYPE " } | "
+.B pppoe_sid
+.IR PSID " | "
+.BR ppp_proto " { " ip " | " ipv6 " | " mpls_uc " | " mpls_mc " | "
+.IR PPP_PROTO " } | "
 .B mpls
 .IR LSE_LIST " | "
 .B mpls_label
@@ -202,7 +206,18 @@ Match on QinQ layer three protocol.
 may be either
 .BR ipv4 ", " ipv6
 or an unsigned 16bit value in hexadecimal format.
-
+.TP
+.BI pppoe_sid " PSID"
+Match on PPPoE session id.
+.I PSID
+is an unsigned 16bit value in decimal format.
+.TP
+.BI ppp_proto " PPP_PROTO"
+Match on PPP layer three protocol.
+.I PPP_PROTO
+may be either
+.BR ip ", " ipv6 ", " mpls_uc ", " mpls_mc
+or an unsigned 16bit value in hexadecimal format.
 .TP
 .BI mpls " LSE_LIST"
 Match on the MPLS label stack.
diff --git a/tc/f_flower.c b/tc/f_flower.c
index 622ec321f310..c95320328b20 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -20,6 +20,7 @@
 #include <linux/ip.h>
 #include <linux/tc_act/tc_vlan.h>
 #include <linux/mpls.h>
+#include <linux/ppp_defs.h>
 
 #include "utils.h"
 #include "tc_util.h"
@@ -55,6 +56,8 @@ static void explain(void)
 		"			cvlan_id VID |\n"
 		"			cvlan_prio PRIORITY |\n"
 		"			cvlan_ethtype [ ipv4 | ipv6 | ETH-TYPE ] |\n"
+		"			pppoe_sid PSID |\n"
+		"			ppp_proto [ ipv4 | ipv6 | mpls_uc | mpls_mc | PPP_PROTO ]"
 		"			dst_mac MASKED-LLADDR |\n"
 		"			src_mac MASKED-LLADDR |\n"
 		"			ip_proto [tcp | udp | sctp | icmp | icmpv6 | IP-PROTO ] |\n"
@@ -1887,6 +1890,43 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
 				fprintf(stderr, "Illegal \"arp_sha\"\n");
 				return -1;
 			}
+
+		} else if (!strcmp(*argv, "pppoe_sid")) {
+			__be16 sid;
+
+			NEXT_ARG();
+			if (eth_type != htons(ETH_P_PPP_SES)) {
+				fprintf(stderr,
+					"Can't set \"pppoe_sid\" if ethertype isn't PPPoE session\n");
+				return -1;
+			}
+			ret = get_be16(&sid, *argv, 10);
+			if (ret < 0 || sid == 0xffff) {
+				fprintf(stderr, "Illegal \"pppoe_sid\"\n");
+				return -1;
+			}
+			addattr16(n, MAX_MSG, TCA_FLOWER_KEY_PPPOE_SID, sid);
+		} else if (!strcmp(*argv, "ppp_proto")) {
+			__be16 proto;
+
+			NEXT_ARG();
+			if (eth_type != htons(ETH_P_PPP_SES)) {
+				fprintf(stderr,
+					"Can't set \"ppp_proto\" if ethertype isn't PPPoE session\n");
+				return -1;
+			}
+			if (ppp_proto_a2n(&proto, *argv))
+				invarg("invalid ppp_proto", *argv);
+			/* get new ethtype for later parsing  */
+			if (proto == htons(PPP_IP))
+				eth_type = htons(ETH_P_IP);
+			else if (proto == htons(PPP_IPV6))
+				eth_type = htons(ETH_P_IPV6);
+			else if (proto == htons(PPP_MPLS_UC))
+				eth_type = htons(ETH_P_MPLS_UC);
+			else if (proto == htons(PPP_MPLS_MC))
+				eth_type = htons(ETH_P_MPLS_MC);
+			addattr16(n, MAX_MSG, TCA_FLOWER_KEY_PPP_PROTO, proto);
 		} else if (matches(*argv, "enc_dst_ip") == 0) {
 			NEXT_ARG();
 			ret = flower_parse_ip_addr(*argv, 0,
@@ -2851,6 +2891,24 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
 	flower_print_eth_addr("arp_tha", tb[TCA_FLOWER_KEY_ARP_THA],
 			      tb[TCA_FLOWER_KEY_ARP_THA_MASK]);
 
+	if (tb[TCA_FLOWER_KEY_PPPOE_SID]) {
+		struct rtattr *attr = tb[TCA_FLOWER_KEY_PPPOE_SID];
+
+		print_nl();
+		print_uint(PRINT_ANY, "pppoe_sid", "  pppoe_sid %u",
+			   rta_getattr_be16(attr));
+	}
+
+	if (tb[TCA_FLOWER_KEY_PPP_PROTO]) {
+		SPRINT_BUF(buf);
+		struct rtattr *attr = tb[TCA_FLOWER_KEY_PPP_PROTO];
+
+		print_nl();
+		print_string(PRINT_ANY, "ppp_proto", "  ppp_proto %s",
+			     ppp_proto_n2a(rta_getattr_u16(attr),
+			     buf, sizeof(buf)));
+	}
+
 	flower_print_ip_addr("enc_dst_ip",
 			     tb[TCA_FLOWER_KEY_ENC_IPV4_DST_MASK] ?
 			     htons(ETH_P_IP) : htons(ETH_P_IPV6),
-- 
2.31.1

Re: [PATCH iproute-next v2 3/3] f_flower: Introduce PPPoE support

[Guillaume Nault]

On Thu, Jul 28, 2022 at 10:44:37AM +0200, Wojciech Drewek wrote:
> Introduce PPPoE specific fields in tc-flower:
> - session id (16 bits)
> - ppp protocol (16 bits)
> Those fields can be provided only when protocol was set to
> ETH_P_PPP_SES. ppp_proto works similar to vlan_ethtype, i.e.
> ppp_proto overwrites eth_type. Thanks to that, fields from
> encapsulated protocols (such as src_ip) can be specified.
> 
> e.g.
>   # tc filter add dev ens6f0 ingress prio 1 protocol ppp_ses \
>       flower \
>         pppoe_sid 1234 \
>         ppp_proto ip \
>         dst_ip 127.0.0.1 \
>         src_ip 127.0.0.2 \
>       action drop
> 
> Vlan and cvlan is also supported, in this case cvlan_ethtype
> or vlan_ethtype has to be set to ETH_P_PPP_SES.
> 
> e.g.
>   # tc filter add dev ens6f0 ingress prio 1 protocol 802.1Q \
>       flower \
>         vlan_id 2 \
>         vlan_ethtype ppp_ses \
>         pppoe_sid 1234 \
>         ppp_proto ip \
>         dst_ip 127.0.0.1 \
>         src_ip 127.0.0.2 \
>       action drop
> 
> Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
> ---
> v2: add pppoe fields to explain
> ---
>  include/uapi/linux/pkt_cls.h |  3 ++
>  man/man8/tc-flower.8         | 17 ++++++++++-
>  tc/f_flower.c                | 58 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 77 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
> index 9a2ee1e39fad..a67dcd8294c9 100644
> --- a/include/uapi/linux/pkt_cls.h
> +++ b/include/uapi/linux/pkt_cls.h
> @@ -589,6 +589,9 @@ enum {
>  
>  	TCA_FLOWER_KEY_NUM_OF_VLANS,    /* u8 */
>  
> +	TCA_FLOWER_KEY_PPPOE_SID,	/* u16 */
> +	TCA_FLOWER_KEY_PPP_PROTO,	/* be16 */
> +
>  	__TCA_FLOWER_MAX,
>  };
>  
> diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
> index 523935242ccf..5e486ea31d37 100644
> --- a/man/man8/tc-flower.8
> +++ b/man/man8/tc-flower.8
> @@ -40,6 +40,10 @@ flower \- flow based traffic control filter
>  .IR PRIORITY " | "
>  .BR cvlan_ethtype " { " ipv4 " | " ipv6 " | "
>  .IR ETH_TYPE " } | "
> +.B pppoe_sid
> +.IR PSID " | "
> +.BR ppp_proto " { " ip " | " ipv6 " | " mpls_uc " | " mpls_mc " | "
> +.IR PPP_PROTO " } | "
>  .B mpls
>  .IR LSE_LIST " | "
>  .B mpls_label
> @@ -202,7 +206,18 @@ Match on QinQ layer three protocol.
>  may be either
>  .BR ipv4 ", " ipv6
>  or an unsigned 16bit value in hexadecimal format.
> -
> +.TP
> +.BI pppoe_sid " PSID"
> +Match on PPPoE session id.
> +.I PSID
> +is an unsigned 16bit value in decimal format.
> +.TP
> +.BI ppp_proto " PPP_PROTO"
> +Match on PPP layer three protocol.
> +.I PPP_PROTO
> +may be either
> +.BR ip ", " ipv6 ", " mpls_uc ", " mpls_mc
> +or an unsigned 16bit value in hexadecimal format.
>  .TP
>  .BI mpls " LSE_LIST"
>  Match on the MPLS label stack.
> diff --git a/tc/f_flower.c b/tc/f_flower.c
> index 622ec321f310..c95320328b20 100644
> --- a/tc/f_flower.c
> +++ b/tc/f_flower.c
> @@ -20,6 +20,7 @@
>  #include <linux/ip.h>
>  #include <linux/tc_act/tc_vlan.h>
>  #include <linux/mpls.h>
> +#include <linux/ppp_defs.h>
>  
>  #include "utils.h"
>  #include "tc_util.h"
> @@ -55,6 +56,8 @@ static void explain(void)
>  		"			cvlan_id VID |\n"
>  		"			cvlan_prio PRIORITY |\n"
>  		"			cvlan_ethtype [ ipv4 | ipv6 | ETH-TYPE ] |\n"
> +		"			pppoe_sid PSID |\n"
> +		"			ppp_proto [ ipv4 | ipv6 | mpls_uc | mpls_mc | PPP_PROTO ]"
>  		"			dst_mac MASKED-LLADDR |\n"
>  		"			src_mac MASKED-LLADDR |\n"
>  		"			ip_proto [tcp | udp | sctp | icmp | icmpv6 | IP-PROTO ] |\n"
> @@ -1887,6 +1890,43 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
>  				fprintf(stderr, "Illegal \"arp_sha\"\n");
>  				return -1;
>  			}
> +
> +		} else if (!strcmp(*argv, "pppoe_sid")) {
> +			__be16 sid;
> +
> +			NEXT_ARG();
> +			if (eth_type != htons(ETH_P_PPP_SES)) {
> +				fprintf(stderr,
> +					"Can't set \"pppoe_sid\" if ethertype isn't PPPoE session\n");
> +				return -1;
> +			}
> +			ret = get_be16(&sid, *argv, 10);
> +			if (ret < 0 || sid == 0xffff) {
> +				fprintf(stderr, "Illegal \"pppoe_sid\"\n");

I don't think it makes sense to reject sid == 0xffff.
One might want to early match and drop such invalid packets.