* [RFC PATCH net-next v4 0/4] ice: PPPoE offload support
@ 2022-07-08 12:24 ` Marcin Szycik
0 siblings, 0 replies; 28+ messages in thread
From: Marcin Szycik @ 2022-07-08 12:24 UTC (permalink / raw)
To: netdev
Cc: anthony.l.nguyen, davem, xiyou.wangcong, jesse.brandeburg,
gustavoars, baowen.zheng, boris.sukholitko, edumazet, kuba, jhs,
jiri, kurt, pablo, pabeni, paulb, simon.horman, komachi.yoshiki,
zhangkaiheb, intel-wired-lan, michal.swiatkowski,
wojciech.drewek, alexandr.lobakin, gnault, mostrows, paulus,
Marcin Szycik
Add support for dissecting PPPoE and PPP-specific fields in flow dissector:
PPPoE session id and PPP protocol type. Add support for those fields in
tc-flower and support offloading PPPoE. Finally, add support for hardware
offload of PPPoE packets in switchdev mode in ice driver.
Example filter:
tc filter add dev $PF1 ingress protocol ppp_ses prio 1 flower pppoe_sid \
1234 ppp_proto ip skip_sw action mirred egress redirect dev $VF1_PR
Changes in iproute2 are required to use the new fields (will be submitted
soon).
ICE COMMS DDP package is required to create a filter in ice.
v4:
* PPPoE header validation
* added MPLS support
* added support for compressed PPP protocol field
* flow_dissector_key_pppoe::session_id stored in __be16
* new field: flow_dissector_key_pppoe::type
* always add an ethtype lookup if PPP/PPPoE options are provided (to
prevent setting incorrect ethtype)
* rebase
v3:
* revert byte order changes in is_ppp_proto_supported from previous
version
* add kernel-doc for is_ppp_proto_supported
* add more CC
v2: cosmetic changes
Marcin Szycik (1):
ice: Add support for PPPoE hardware offload
Wojciech Drewek (3):
flow_dissector: Add PPPoE dissectors
net/sched: flower: Add PPPoE filter
flow_offload: Introduce flow_match_pppoe
drivers/net/ethernet/intel/ice/ice.h | 1 +
.../net/ethernet/intel/ice/ice_flex_pipe.c | 5 +-
.../ethernet/intel/ice/ice_protocol_type.h | 11 ++
drivers/net/ethernet/intel/ice/ice_switch.c | 165 ++++++++++++++++++
drivers/net/ethernet/intel/ice/ice_tc_lib.c | 71 +++++++-
drivers/net/ethernet/intel/ice/ice_tc_lib.h | 8 +
include/net/flow_dissector.h | 13 ++
include/net/flow_offload.h | 6 +
include/uapi/linux/pkt_cls.h | 3 +
net/core/flow_dissector.c | 84 ++++++++-
net/core/flow_offload.c | 7 +
net/sched/cls_flower.c | 61 +++++++
12 files changed, 426 insertions(+), 9 deletions(-)
--
2.35.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Intel-wired-lan] [RFC PATCH net-next v4 0/4] ice: PPPoE offload support
@ 2022-07-08 12:24 ` Marcin Szycik
0 siblings, 0 replies; 28+ messages in thread
From: Marcin Szycik @ 2022-07-08 12:24 UTC (permalink / raw)
To: netdev
Cc: simon.horman, kurt, paulb, edumazet, boris.sukholitko, jiri,
paulus, gnault, jesse.brandeburg, intel-wired-lan, kuba,
zhangkaiheb, pablo, baowen.zheng, komachi.yoshiki, jhs, mostrows,
xiyou.wangcong, pabeni, gustavoars, davem
Add support for dissecting PPPoE and PPP-specific fields in flow dissector:
PPPoE session id and PPP protocol type. Add support for those fields in
tc-flower and support offloading PPPoE. Finally, add support for hardware
offload of PPPoE packets in switchdev mode in ice driver.
Example filter:
tc filter add dev $PF1 ingress protocol ppp_ses prio 1 flower pppoe_sid \
1234 ppp_proto ip skip_sw action mirred egress redirect dev $VF1_PR
Changes in iproute2 are required to use the new fields (will be submitted
soon).
ICE COMMS DDP package is required to create a filter in ice.
v4:
* PPPoE header validation
* added MPLS support
* added support for compressed PPP protocol field
* flow_dissector_key_pppoe::session_id stored in __be16
* new field: flow_dissector_key_pppoe::type
* always add an ethtype lookup if PPP/PPPoE options are provided (to
prevent setting incorrect ethtype)
* rebase
v3:
* revert byte order changes in is_ppp_proto_supported from previous
version
* add kernel-doc for is_ppp_proto_supported
* add more CC
v2: cosmetic changes
Marcin Szycik (1):
ice: Add support for PPPoE hardware offload
Wojciech Drewek (3):
flow_dissector: Add PPPoE dissectors
net/sched: flower: Add PPPoE filter
flow_offload: Introduce flow_match_pppoe
drivers/net/ethernet/intel/ice/ice.h | 1 +
.../net/ethernet/intel/ice/ice_flex_pipe.c | 5 +-
.../ethernet/intel/ice/ice_protocol_type.h | 11 ++
drivers/net/ethernet/intel/ice/ice_switch.c | 165 ++++++++++++++++++
drivers/net/ethernet/intel/ice/ice_tc_lib.c | 71 +++++++-
drivers/net/ethernet/intel/ice/ice_tc_lib.h | 8 +
include/net/flow_dissector.h | 13 ++
include/net/flow_offload.h | 6 +
include/uapi/linux/pkt_cls.h | 3 +
net/core/flow_dissector.c | 84 ++++++++-
net/core/flow_offload.c | 7 +
net/sched/cls_flower.c | 61 +++++++
12 files changed, 426 insertions(+), 9 deletions(-)
--
2.35.1
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 28+ messages in thread
* [RFC PATCH net-next v4 1/4] flow_dissector: Add PPPoE dissectors
2022-07-08 12:24 ` [Intel-wired-lan] " Marcin Szycik
@ 2022-07-08 12:24 ` Marcin Szycik
-1 siblings, 0 replies; 28+ messages in thread
From: Marcin Szycik @ 2022-07-08 12:24 UTC (permalink / raw)
To: netdev
Cc: anthony.l.nguyen, davem, xiyou.wangcong, jesse.brandeburg,
gustavoars, baowen.zheng, boris.sukholitko, edumazet, kuba, jhs,
jiri, kurt, pablo, pabeni, paulb, simon.horman, komachi.yoshiki,
zhangkaiheb, intel-wired-lan, michal.swiatkowski,
wojciech.drewek, alexandr.lobakin, gnault, mostrows, paulus
From: Wojciech Drewek <wojciech.drewek@intel.com>
Allow to dissect PPPoE specific fields which are:
- session ID (16 bits)
- ppp protocol (16 bits)
- type (16 bits) - this is PPPoE ethertype, for now only
ETH_P_PPP_SES is supported, possible ETH_P_PPP_DISC
in the future
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>
---
v4:
* pppoe header validation
* added MPLS dissection
* added support for compressed ppp protocol field
* flow_dissector_key_pppoe::session_id stored in __be16
* new field: flow_dissector_key_pppoe::type
v3: revert byte order changes in is_ppp_proto_supported from
previous version
v2: ntohs instead of htons in is_ppp_proto_supported
include/net/flow_dissector.h | 13 ++++++
net/core/flow_dissector.c | 84 +++++++++++++++++++++++++++++++++---
2 files changed, 90 insertions(+), 7 deletions(-)
diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
index a4c6057c7097..af0d429b9a26 100644
--- a/include/net/flow_dissector.h
+++ b/include/net/flow_dissector.h
@@ -261,6 +261,18 @@ 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
+ * @type: pppoe eth type
+ */
+struct flow_dissector_key_pppoe {
+ __be16 session_id;
+ __be16 ppp_proto;
+ __be16 type;
+};
+
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 +303,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..3a90219d2354 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -895,6 +895,42 @@ bool bpf_flow_dissect(struct bpf_prog *prog, struct bpf_flow_dissector *ctx,
return result == BPF_OK;
}
+/**
+ * is_ppp_proto_supported - checks if inner PPP protocol should be dissected
+ * @proto: protocol type (PPP proto field)
+ */
+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_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;
+ }
+}
+
+static bool is_pppoe_ses_hdr_valid(struct pppoe_hdr hdr)
+{
+ return hdr.ver == 1 && hdr.type == 1 && hdr.code == 0;
+}
+
/**
* __skb_flow_dissect - extract the flow_keys struct and return it
* @net: associated network namespace, derived from @skb if NULL
@@ -1214,26 +1250,60 @@ bool __skb_flow_dissect(const struct net *net,
struct pppoe_hdr hdr;
__be16 proto;
} *hdr, _hdr;
+ __be16 ppp_proto;
+
hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr);
if (!hdr) {
fdret = FLOW_DISSECT_RET_OUT_BAD;
break;
}
- nhoff += PPPOE_SES_HLEN;
- switch (hdr->proto) {
- case htons(PPP_IP):
+ if (!is_pppoe_ses_hdr_valid(hdr->hdr)) {
+ fdret = FLOW_DISSECT_RET_OUT_BAD;
+ break;
+ }
+
+ /* least significant bit of the first byte
+ * indicates if protocol field was compressed
+ */
+ if (hdr->proto & 1) {
+ ppp_proto = hdr->proto << 8;
+ nhoff += PPPOE_SES_HLEN - 1;
+ } else {
+ ppp_proto = hdr->proto;
+ nhoff += PPPOE_SES_HLEN;
+ }
+
+ if (ppp_proto == htons(PPP_IP)) {
proto = htons(ETH_P_IP);
fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
- break;
- case htons(PPP_IPV6):
+ } else if (ppp_proto == htons(PPP_IPV6)) {
proto = htons(ETH_P_IPV6);
fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
- break;
- default:
+ } else if (ppp_proto == htons(PPP_MPLS_UC)) {
+ proto = htons(ETH_P_MPLS_UC);
+ fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
+ } else if (ppp_proto == htons(PPP_MPLS_MC)) {
+ proto = htons(ETH_P_MPLS_MC);
+ fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
+ } else if (is_ppp_proto_supported(ppp_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 = hdr->hdr.sid;
+ key_pppoe->ppp_proto = ppp_proto;
+ key_pppoe->type = htons(ETH_P_PPP_SES);
+ }
break;
}
case htons(ETH_P_TIPC): {
--
2.35.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Intel-wired-lan] [RFC PATCH net-next v4 1/4] flow_dissector: Add PPPoE dissectors
@ 2022-07-08 12:24 ` Marcin Szycik
0 siblings, 0 replies; 28+ messages in thread
From: Marcin Szycik @ 2022-07-08 12:24 UTC (permalink / raw)
To: netdev
Cc: simon.horman, kurt, paulb, edumazet, boris.sukholitko, jiri,
paulus, gnault, jesse.brandeburg, intel-wired-lan, kuba,
zhangkaiheb, pablo, baowen.zheng, komachi.yoshiki, jhs, mostrows,
xiyou.wangcong, pabeni, gustavoars, davem
From: Wojciech Drewek <wojciech.drewek@intel.com>
Allow to dissect PPPoE specific fields which are:
- session ID (16 bits)
- ppp protocol (16 bits)
- type (16 bits) - this is PPPoE ethertype, for now only
ETH_P_PPP_SES is supported, possible ETH_P_PPP_DISC
in the future
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>
---
v4:
* pppoe header validation
* added MPLS dissection
* added support for compressed ppp protocol field
* flow_dissector_key_pppoe::session_id stored in __be16
* new field: flow_dissector_key_pppoe::type
v3: revert byte order changes in is_ppp_proto_supported from
previous version
v2: ntohs instead of htons in is_ppp_proto_supported
include/net/flow_dissector.h | 13 ++++++
net/core/flow_dissector.c | 84 +++++++++++++++++++++++++++++++++---
2 files changed, 90 insertions(+), 7 deletions(-)
diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
index a4c6057c7097..af0d429b9a26 100644
--- a/include/net/flow_dissector.h
+++ b/include/net/flow_dissector.h
@@ -261,6 +261,18 @@ 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
+ * @type: pppoe eth type
+ */
+struct flow_dissector_key_pppoe {
+ __be16 session_id;
+ __be16 ppp_proto;
+ __be16 type;
+};
+
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 +303,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..3a90219d2354 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -895,6 +895,42 @@ bool bpf_flow_dissect(struct bpf_prog *prog, struct bpf_flow_dissector *ctx,
return result == BPF_OK;
}
+/**
+ * is_ppp_proto_supported - checks if inner PPP protocol should be dissected
+ * @proto: protocol type (PPP proto field)
+ */
+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_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;
+ }
+}
+
+static bool is_pppoe_ses_hdr_valid(struct pppoe_hdr hdr)
+{
+ return hdr.ver == 1 && hdr.type == 1 && hdr.code == 0;
+}
+
/**
* __skb_flow_dissect - extract the flow_keys struct and return it
* @net: associated network namespace, derived from @skb if NULL
@@ -1214,26 +1250,60 @@ bool __skb_flow_dissect(const struct net *net,
struct pppoe_hdr hdr;
__be16 proto;
} *hdr, _hdr;
+ __be16 ppp_proto;
+
hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr);
if (!hdr) {
fdret = FLOW_DISSECT_RET_OUT_BAD;
break;
}
- nhoff += PPPOE_SES_HLEN;
- switch (hdr->proto) {
- case htons(PPP_IP):
+ if (!is_pppoe_ses_hdr_valid(hdr->hdr)) {
+ fdret = FLOW_DISSECT_RET_OUT_BAD;
+ break;
+ }
+
+ /* least significant bit of the first byte
+ * indicates if protocol field was compressed
+ */
+ if (hdr->proto & 1) {
+ ppp_proto = hdr->proto << 8;
+ nhoff += PPPOE_SES_HLEN - 1;
+ } else {
+ ppp_proto = hdr->proto;
+ nhoff += PPPOE_SES_HLEN;
+ }
+
+ if (ppp_proto == htons(PPP_IP)) {
proto = htons(ETH_P_IP);
fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
- break;
- case htons(PPP_IPV6):
+ } else if (ppp_proto == htons(PPP_IPV6)) {
proto = htons(ETH_P_IPV6);
fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
- break;
- default:
+ } else if (ppp_proto == htons(PPP_MPLS_UC)) {
+ proto = htons(ETH_P_MPLS_UC);
+ fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
+ } else if (ppp_proto == htons(PPP_MPLS_MC)) {
+ proto = htons(ETH_P_MPLS_MC);
+ fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
+ } else if (is_ppp_proto_supported(ppp_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 = hdr->hdr.sid;
+ key_pppoe->ppp_proto = ppp_proto;
+ key_pppoe->type = htons(ETH_P_PPP_SES);
+ }
break;
}
case htons(ETH_P_TIPC): {
--
2.35.1
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [RFC PATCH net-next v4 2/4] net/sched: flower: Add PPPoE filter
2022-07-08 12:24 ` [Intel-wired-lan] " Marcin Szycik
@ 2022-07-08 12:24 ` Marcin Szycik
-1 siblings, 0 replies; 28+ messages in thread
From: Marcin Szycik @ 2022-07-08 12:24 UTC (permalink / raw)
To: netdev
Cc: anthony.l.nguyen, davem, xiyou.wangcong, jesse.brandeburg,
gustavoars, baowen.zheng, boris.sukholitko, edumazet, kuba, jhs,
jiri, kurt, pablo, pabeni, paulb, simon.horman, komachi.yoshiki,
zhangkaiheb, intel-wired-lan, michal.swiatkowski,
wojciech.drewek, alexandr.lobakin, gnault, mostrows, paulus
From: Wojciech Drewek <wojciech.drewek@intel.com>
Add support for PPPoE specific fields for tc-flower.
Those fields can be provided only when protocol was set
to ETH_P_PPP_SES. Defines, dump, load and set are being done here.
Overwrite basic.n_proto only in case of PPP_IP and PPP_IPV6,
otherwise leave it as ETH_P_PPP_SES.
Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
---
v4:
* support of MPLS inner fields
* session_id stored in __be16
include/uapi/linux/pkt_cls.h | 3 ++
net/sched/cls_flower.c | 61 ++++++++++++++++++++++++++++++++++++
2 files changed, 64 insertions(+)
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index 9a2ee1e39fad..c142c0f8ed8a 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, /* be16 */
+ TCA_FLOWER_KEY_PPP_PROTO, /* be16 */
+
__TCA_FLOWER_MAX,
};
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index dcca70144dff..2a0ebad0e61c 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -16,6 +16,7 @@
#include <linux/in6.h>
#include <linux/ip.h>
#include <linux/mpls.h>
+#include <linux/ppp_defs.h>
#include <net/sch_generic.h>
#include <net/pkt_cls.h>
@@ -73,6 +74,7 @@ struct fl_flow_key {
struct flow_dissector_key_ct ct;
struct flow_dissector_key_hash hash;
struct flow_dissector_key_num_of_vlans num_of_vlans;
+ struct flow_dissector_key_pppoe pppoe;
} __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */
struct fl_flow_mask_range {
@@ -714,6 +716,8 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
[TCA_FLOWER_KEY_HASH] = { .type = NLA_U32 },
[TCA_FLOWER_KEY_HASH_MASK] = { .type = NLA_U32 },
[TCA_FLOWER_KEY_NUM_OF_VLANS] = { .type = NLA_U8 },
+ [TCA_FLOWER_KEY_PPPOE_SID] = { .type = NLA_U16 },
+ [TCA_FLOWER_KEY_PPP_PROTO] = { .type = NLA_U16 },
};
@@ -1041,6 +1045,50 @@ static void fl_set_key_vlan(struct nlattr **tb,
}
}
+static void fl_set_key_pppoe(struct nlattr **tb,
+ struct flow_dissector_key_pppoe *key_val,
+ struct flow_dissector_key_pppoe *key_mask,
+ struct fl_flow_key *key,
+ struct fl_flow_key *mask)
+{
+ /* key_val::type must be set to ETH_P_PPP_SES
+ * because ETH_P_PPP_SES was stored in basic.n_proto
+ * which might get overwritten by ppp_proto
+ * or might be set to 0, the role of key_val::type
+ * is simmilar to vlan_key::tpid
+ */
+ key_val->type = htons(ETH_P_PPP_SES);
+ key_mask->type = cpu_to_be16(~0);
+
+ if (tb[TCA_FLOWER_KEY_PPPOE_SID]) {
+ key_val->session_id =
+ nla_get_be16(tb[TCA_FLOWER_KEY_PPPOE_SID]);
+ key_mask->session_id = cpu_to_be16(~0);
+ }
+ if (tb[TCA_FLOWER_KEY_PPP_PROTO]) {
+ key_val->ppp_proto =
+ nla_get_be16(tb[TCA_FLOWER_KEY_PPP_PROTO]);
+ key_mask->ppp_proto = cpu_to_be16(~0);
+
+ if (key_val->ppp_proto == htons(PPP_IP)) {
+ key->basic.n_proto = htons(ETH_P_IP);
+ mask->basic.n_proto = cpu_to_be16(~0);
+ } else if (key_val->ppp_proto == htons(PPP_IPV6)) {
+ key->basic.n_proto = htons(ETH_P_IPV6);
+ mask->basic.n_proto = cpu_to_be16(~0);
+ } else if (key_val->ppp_proto == htons(PPP_MPLS_UC)) {
+ key->basic.n_proto = htons(ETH_P_MPLS_UC);
+ mask->basic.n_proto = cpu_to_be16(~0);
+ } else if (key_val->ppp_proto == htons(PPP_MPLS_MC)) {
+ key->basic.n_proto = htons(ETH_P_MPLS_MC);
+ mask->basic.n_proto = cpu_to_be16(~0);
+ }
+ } else {
+ key->basic.n_proto = 0;
+ mask->basic.n_proto = cpu_to_be16(0);
+ }
+}
+
static void fl_set_key_flag(u32 flower_key, u32 flower_mask,
u32 *dissector_key, u32 *dissector_mask,
u32 flower_flag_bit, u32 dissector_flag_bit)
@@ -1651,6 +1699,9 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
}
}
+ if (key->basic.n_proto == htons(ETH_P_PPP_SES))
+ fl_set_key_pppoe(tb, &key->pppoe, &mask->pppoe, key, mask);
+
if (key->basic.n_proto == htons(ETH_P_IP) ||
key->basic.n_proto == htons(ETH_P_IPV6)) {
fl_set_key_val(tb, &key->basic.ip_proto, TCA_FLOWER_KEY_IP_PROTO,
@@ -1923,6 +1974,8 @@ static void fl_init_dissector(struct flow_dissector *dissector,
FLOW_DISSECTOR_KEY_HASH, hash);
FL_KEY_SET_IF_MASKED(mask, keys, cnt,
FLOW_DISSECTOR_KEY_NUM_OF_VLANS, num_of_vlans);
+ FL_KEY_SET_IF_MASKED(mask, keys, cnt,
+ FLOW_DISSECTOR_KEY_PPPOE, pppoe);
skb_flow_dissector_init(dissector, keys, cnt);
}
@@ -3051,6 +3104,14 @@ static int fl_dump_key(struct sk_buff *skb, struct net *net,
fl_dump_key_ip(skb, false, &key->ip, &mask->ip)))
goto nla_put_failure;
+ if (mask->pppoe.session_id &&
+ nla_put_be16(skb, TCA_FLOWER_KEY_PPPOE_SID, key->pppoe.session_id))
+ goto nla_put_failure;
+
+ if (mask->basic.n_proto && mask->pppoe.ppp_proto &&
+ nla_put_be16(skb, TCA_FLOWER_KEY_PPP_PROTO, key->pppoe.ppp_proto))
+ goto nla_put_failure;
+
if (key->control.addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS &&
(fl_dump_key_val(skb, &key->ipv4.src, TCA_FLOWER_KEY_IPV4_SRC,
&mask->ipv4.src, TCA_FLOWER_KEY_IPV4_SRC_MASK,
--
2.35.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Intel-wired-lan] [RFC PATCH net-next v4 2/4] net/sched: flower: Add PPPoE filter
@ 2022-07-08 12:24 ` Marcin Szycik
0 siblings, 0 replies; 28+ messages in thread
From: Marcin Szycik @ 2022-07-08 12:24 UTC (permalink / raw)
To: netdev
Cc: simon.horman, kurt, paulb, edumazet, boris.sukholitko, jiri,
paulus, gnault, jesse.brandeburg, intel-wired-lan, kuba,
zhangkaiheb, pablo, baowen.zheng, komachi.yoshiki, jhs, mostrows,
xiyou.wangcong, pabeni, gustavoars, davem
From: Wojciech Drewek <wojciech.drewek@intel.com>
Add support for PPPoE specific fields for tc-flower.
Those fields can be provided only when protocol was set
to ETH_P_PPP_SES. Defines, dump, load and set are being done here.
Overwrite basic.n_proto only in case of PPP_IP and PPP_IPV6,
otherwise leave it as ETH_P_PPP_SES.
Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
---
v4:
* support of MPLS inner fields
* session_id stored in __be16
include/uapi/linux/pkt_cls.h | 3 ++
net/sched/cls_flower.c | 61 ++++++++++++++++++++++++++++++++++++
2 files changed, 64 insertions(+)
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index 9a2ee1e39fad..c142c0f8ed8a 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, /* be16 */
+ TCA_FLOWER_KEY_PPP_PROTO, /* be16 */
+
__TCA_FLOWER_MAX,
};
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index dcca70144dff..2a0ebad0e61c 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -16,6 +16,7 @@
#include <linux/in6.h>
#include <linux/ip.h>
#include <linux/mpls.h>
+#include <linux/ppp_defs.h>
#include <net/sch_generic.h>
#include <net/pkt_cls.h>
@@ -73,6 +74,7 @@ struct fl_flow_key {
struct flow_dissector_key_ct ct;
struct flow_dissector_key_hash hash;
struct flow_dissector_key_num_of_vlans num_of_vlans;
+ struct flow_dissector_key_pppoe pppoe;
} __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */
struct fl_flow_mask_range {
@@ -714,6 +716,8 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
[TCA_FLOWER_KEY_HASH] = { .type = NLA_U32 },
[TCA_FLOWER_KEY_HASH_MASK] = { .type = NLA_U32 },
[TCA_FLOWER_KEY_NUM_OF_VLANS] = { .type = NLA_U8 },
+ [TCA_FLOWER_KEY_PPPOE_SID] = { .type = NLA_U16 },
+ [TCA_FLOWER_KEY_PPP_PROTO] = { .type = NLA_U16 },
};
@@ -1041,6 +1045,50 @@ static void fl_set_key_vlan(struct nlattr **tb,
}
}
+static void fl_set_key_pppoe(struct nlattr **tb,
+ struct flow_dissector_key_pppoe *key_val,
+ struct flow_dissector_key_pppoe *key_mask,
+ struct fl_flow_key *key,
+ struct fl_flow_key *mask)
+{
+ /* key_val::type must be set to ETH_P_PPP_SES
+ * because ETH_P_PPP_SES was stored in basic.n_proto
+ * which might get overwritten by ppp_proto
+ * or might be set to 0, the role of key_val::type
+ * is simmilar to vlan_key::tpid
+ */
+ key_val->type = htons(ETH_P_PPP_SES);
+ key_mask->type = cpu_to_be16(~0);
+
+ if (tb[TCA_FLOWER_KEY_PPPOE_SID]) {
+ key_val->session_id =
+ nla_get_be16(tb[TCA_FLOWER_KEY_PPPOE_SID]);
+ key_mask->session_id = cpu_to_be16(~0);
+ }
+ if (tb[TCA_FLOWER_KEY_PPP_PROTO]) {
+ key_val->ppp_proto =
+ nla_get_be16(tb[TCA_FLOWER_KEY_PPP_PROTO]);
+ key_mask->ppp_proto = cpu_to_be16(~0);
+
+ if (key_val->ppp_proto == htons(PPP_IP)) {
+ key->basic.n_proto = htons(ETH_P_IP);
+ mask->basic.n_proto = cpu_to_be16(~0);
+ } else if (key_val->ppp_proto == htons(PPP_IPV6)) {
+ key->basic.n_proto = htons(ETH_P_IPV6);
+ mask->basic.n_proto = cpu_to_be16(~0);
+ } else if (key_val->ppp_proto == htons(PPP_MPLS_UC)) {
+ key->basic.n_proto = htons(ETH_P_MPLS_UC);
+ mask->basic.n_proto = cpu_to_be16(~0);
+ } else if (key_val->ppp_proto == htons(PPP_MPLS_MC)) {
+ key->basic.n_proto = htons(ETH_P_MPLS_MC);
+ mask->basic.n_proto = cpu_to_be16(~0);
+ }
+ } else {
+ key->basic.n_proto = 0;
+ mask->basic.n_proto = cpu_to_be16(0);
+ }
+}
+
static void fl_set_key_flag(u32 flower_key, u32 flower_mask,
u32 *dissector_key, u32 *dissector_mask,
u32 flower_flag_bit, u32 dissector_flag_bit)
@@ -1651,6 +1699,9 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
}
}
+ if (key->basic.n_proto == htons(ETH_P_PPP_SES))
+ fl_set_key_pppoe(tb, &key->pppoe, &mask->pppoe, key, mask);
+
if (key->basic.n_proto == htons(ETH_P_IP) ||
key->basic.n_proto == htons(ETH_P_IPV6)) {
fl_set_key_val(tb, &key->basic.ip_proto, TCA_FLOWER_KEY_IP_PROTO,
@@ -1923,6 +1974,8 @@ static void fl_init_dissector(struct flow_dissector *dissector,
FLOW_DISSECTOR_KEY_HASH, hash);
FL_KEY_SET_IF_MASKED(mask, keys, cnt,
FLOW_DISSECTOR_KEY_NUM_OF_VLANS, num_of_vlans);
+ FL_KEY_SET_IF_MASKED(mask, keys, cnt,
+ FLOW_DISSECTOR_KEY_PPPOE, pppoe);
skb_flow_dissector_init(dissector, keys, cnt);
}
@@ -3051,6 +3104,14 @@ static int fl_dump_key(struct sk_buff *skb, struct net *net,
fl_dump_key_ip(skb, false, &key->ip, &mask->ip)))
goto nla_put_failure;
+ if (mask->pppoe.session_id &&
+ nla_put_be16(skb, TCA_FLOWER_KEY_PPPOE_SID, key->pppoe.session_id))
+ goto nla_put_failure;
+
+ if (mask->basic.n_proto && mask->pppoe.ppp_proto &&
+ nla_put_be16(skb, TCA_FLOWER_KEY_PPP_PROTO, key->pppoe.ppp_proto))
+ goto nla_put_failure;
+
if (key->control.addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS &&
(fl_dump_key_val(skb, &key->ipv4.src, TCA_FLOWER_KEY_IPV4_SRC,
&mask->ipv4.src, TCA_FLOWER_KEY_IPV4_SRC_MASK,
--
2.35.1
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [RFC PATCH net-next v4 3/4] flow_offload: Introduce flow_match_pppoe
2022-07-08 12:24 ` [Intel-wired-lan] " Marcin Szycik
@ 2022-07-08 12:24 ` Marcin Szycik
-1 siblings, 0 replies; 28+ messages in thread
From: Marcin Szycik @ 2022-07-08 12:24 UTC (permalink / raw)
To: netdev
Cc: anthony.l.nguyen, davem, xiyou.wangcong, jesse.brandeburg,
gustavoars, baowen.zheng, boris.sukholitko, edumazet, kuba, jhs,
jiri, kurt, pablo, pabeni, paulb, simon.horman, komachi.yoshiki,
zhangkaiheb, intel-wired-lan, michal.swiatkowski,
wojciech.drewek, alexandr.lobakin, gnault, mostrows, paulus
From: Wojciech Drewek <wojciech.drewek@intel.com>
Allow to offload PPPoE filters by adding flow_rule_match_pppoe.
Drivers can extract PPPoE specific fields from now on.
Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
---
include/net/flow_offload.h | 6 ++++++
net/core/flow_offload.c | 7 +++++++
2 files changed, 13 insertions(+)
diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
index 7ac313858037..8826606c8464 100644
--- a/include/net/flow_offload.h
+++ b/include/net/flow_offload.h
@@ -72,6 +72,10 @@ struct flow_match_ct {
struct flow_dissector_key_ct *key, *mask;
};
+struct flow_match_pppoe {
+ struct flow_dissector_key_pppoe *key, *mask;
+};
+
struct flow_rule;
void flow_rule_match_meta(const struct flow_rule *rule,
@@ -116,6 +120,8 @@ void flow_rule_match_enc_opts(const struct flow_rule *rule,
struct flow_match_enc_opts *out);
void flow_rule_match_ct(const struct flow_rule *rule,
struct flow_match_ct *out);
+void flow_rule_match_pppoe(const struct flow_rule *rule,
+ struct flow_match_pppoe *out);
enum flow_action_id {
FLOW_ACTION_ACCEPT = 0,
diff --git a/net/core/flow_offload.c b/net/core/flow_offload.c
index 929f6379a279..d13d293e3de2 100644
--- a/net/core/flow_offload.c
+++ b/net/core/flow_offload.c
@@ -223,6 +223,13 @@ void flow_rule_match_ct(const struct flow_rule *rule,
}
EXPORT_SYMBOL(flow_rule_match_ct);
+void flow_rule_match_pppoe(const struct flow_rule *rule,
+ struct flow_match_pppoe *out)
+{
+ FLOW_DISSECTOR_MATCH(rule, FLOW_DISSECTOR_KEY_PPPOE, out);
+}
+EXPORT_SYMBOL(flow_rule_match_pppoe);
+
struct flow_block_cb *flow_block_cb_alloc(flow_setup_cb_t *cb,
void *cb_ident, void *cb_priv,
void (*release)(void *cb_priv))
--
2.35.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Intel-wired-lan] [RFC PATCH net-next v4 3/4] flow_offload: Introduce flow_match_pppoe
@ 2022-07-08 12:24 ` Marcin Szycik
0 siblings, 0 replies; 28+ messages in thread
From: Marcin Szycik @ 2022-07-08 12:24 UTC (permalink / raw)
To: netdev
Cc: simon.horman, kurt, paulb, edumazet, boris.sukholitko, jiri,
paulus, gnault, jesse.brandeburg, intel-wired-lan, kuba,
zhangkaiheb, pablo, baowen.zheng, komachi.yoshiki, jhs, mostrows,
xiyou.wangcong, pabeni, gustavoars, davem
From: Wojciech Drewek <wojciech.drewek@intel.com>
Allow to offload PPPoE filters by adding flow_rule_match_pppoe.
Drivers can extract PPPoE specific fields from now on.
Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
---
include/net/flow_offload.h | 6 ++++++
net/core/flow_offload.c | 7 +++++++
2 files changed, 13 insertions(+)
diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
index 7ac313858037..8826606c8464 100644
--- a/include/net/flow_offload.h
+++ b/include/net/flow_offload.h
@@ -72,6 +72,10 @@ struct flow_match_ct {
struct flow_dissector_key_ct *key, *mask;
};
+struct flow_match_pppoe {
+ struct flow_dissector_key_pppoe *key, *mask;
+};
+
struct flow_rule;
void flow_rule_match_meta(const struct flow_rule *rule,
@@ -116,6 +120,8 @@ void flow_rule_match_enc_opts(const struct flow_rule *rule,
struct flow_match_enc_opts *out);
void flow_rule_match_ct(const struct flow_rule *rule,
struct flow_match_ct *out);
+void flow_rule_match_pppoe(const struct flow_rule *rule,
+ struct flow_match_pppoe *out);
enum flow_action_id {
FLOW_ACTION_ACCEPT = 0,
diff --git a/net/core/flow_offload.c b/net/core/flow_offload.c
index 929f6379a279..d13d293e3de2 100644
--- a/net/core/flow_offload.c
+++ b/net/core/flow_offload.c
@@ -223,6 +223,13 @@ void flow_rule_match_ct(const struct flow_rule *rule,
}
EXPORT_SYMBOL(flow_rule_match_ct);
+void flow_rule_match_pppoe(const struct flow_rule *rule,
+ struct flow_match_pppoe *out)
+{
+ FLOW_DISSECTOR_MATCH(rule, FLOW_DISSECTOR_KEY_PPPOE, out);
+}
+EXPORT_SYMBOL(flow_rule_match_pppoe);
+
struct flow_block_cb *flow_block_cb_alloc(flow_setup_cb_t *cb,
void *cb_ident, void *cb_priv,
void (*release)(void *cb_priv))
--
2.35.1
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [RFC PATCH net-next v4 4/4] ice: Add support for PPPoE hardware offload
2022-07-08 12:24 ` [Intel-wired-lan] " Marcin Szycik
@ 2022-07-08 12:24 ` Marcin Szycik
-1 siblings, 0 replies; 28+ messages in thread
From: Marcin Szycik @ 2022-07-08 12:24 UTC (permalink / raw)
To: netdev
Cc: anthony.l.nguyen, davem, xiyou.wangcong, jesse.brandeburg,
gustavoars, baowen.zheng, boris.sukholitko, edumazet, kuba, jhs,
jiri, kurt, pablo, pabeni, paulb, simon.horman, komachi.yoshiki,
zhangkaiheb, intel-wired-lan, michal.swiatkowski,
wojciech.drewek, alexandr.lobakin, gnault, mostrows, paulus,
Marcin Szycik
Add support for creating PPPoE filters in switchdev mode. Add support
for parsing PPPoE and PPP-specific tc options: pppoe_sid and ppp_proto.
Example filter:
tc filter add dev $PF1 ingress protocol ppp_ses prio 1 flower pppoe_sid \
1234 ppp_proto ip skip_sw action mirred egress redirect dev $VF1_PR
Changes in iproute2 are required to use the new fields.
ICE COMMS DDP package is required to create a filter as it contains PPPoE
profiles. Added a warning message when loaded DDP package does not contain
required profiles.
Signed-off-by: Marcin Szycik <marcin.szycik@linux.intel.com>
---
v4:
* adapted code to the redefined ice_tc_pppoe_hdr (session_id is now
stored in __be16)
* removed note about missing support for vlan fields:
263957263a00 ("ice: switch: dynamically add VLAN headers to dummy packets")
is already accepted
* always add an ethtype lookup if PPP/PPPoE options are provided (to
prevent setting incorrect ethtype)
* rebase
v2: wrap one long line
drivers/net/ethernet/intel/ice/ice.h | 1 +
.../net/ethernet/intel/ice/ice_flex_pipe.c | 5 +-
.../ethernet/intel/ice/ice_protocol_type.h | 11 ++
drivers/net/ethernet/intel/ice/ice_switch.c | 165 ++++++++++++++++++
drivers/net/ethernet/intel/ice/ice_tc_lib.c | 71 +++++++-
drivers/net/ethernet/intel/ice/ice_tc_lib.h | 8 +
6 files changed, 259 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index 60453b3b8d23..0e48a930f004 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -52,6 +52,7 @@
#include <net/udp_tunnel.h>
#include <net/vxlan.h>
#include <net/gtp.h>
+#include <linux/ppp_defs.h>
#include "ice_devids.h"
#include "ice_type.h"
#include "ice_txrx.h"
diff --git a/drivers/net/ethernet/intel/ice/ice_flex_pipe.c b/drivers/net/ethernet/intel/ice/ice_flex_pipe.c
index ada5198b5b16..4b3bb19e1d06 100644
--- a/drivers/net/ethernet/intel/ice/ice_flex_pipe.c
+++ b/drivers/net/ethernet/intel/ice/ice_flex_pipe.c
@@ -1964,8 +1964,11 @@ ice_get_sw_fv_list(struct ice_hw *hw, struct ice_prot_lkup_ext *lkups,
}
}
} while (fv);
- if (list_empty(fv_list))
+ if (list_empty(fv_list)) {
+ dev_warn(ice_hw_to_dev(hw), "Required profiles not found in currently loaded DDP package");
return -EIO;
+ }
+
return 0;
err:
diff --git a/drivers/net/ethernet/intel/ice/ice_protocol_type.h b/drivers/net/ethernet/intel/ice/ice_protocol_type.h
index d4a0d089649c..560efc7654c7 100644
--- a/drivers/net/ethernet/intel/ice/ice_protocol_type.h
+++ b/drivers/net/ethernet/intel/ice/ice_protocol_type.h
@@ -43,6 +43,7 @@ enum ice_protocol_type {
ICE_NVGRE,
ICE_GTP,
ICE_GTP_NO_PAY,
+ ICE_PPPOE,
ICE_VLAN_EX,
ICE_VLAN_IN,
ICE_VXLAN_GPE,
@@ -109,6 +110,7 @@ enum ice_prot_id {
#define ICE_TCP_IL_HW 49
#define ICE_UDP_ILOS_HW 53
#define ICE_GRE_OF_HW 64
+#define ICE_PPPOE_HW 103
#define ICE_UDP_OF_HW 52 /* UDP Tunnels */
#define ICE_META_DATA_ID_HW 255 /* this is used for tunnel and VLAN type */
@@ -207,6 +209,14 @@ struct ice_udp_gtp_hdr {
u8 rsvrd;
};
+struct ice_pppoe_hdr {
+ u8 rsrvd_ver_type;
+ u8 rsrvd_code;
+ __be16 session_id;
+ __be16 length;
+ __be16 ppp_prot_id; /* control and data only */
+};
+
struct ice_nvgre_hdr {
__be16 flags;
__be16 protocol;
@@ -224,6 +234,7 @@ union ice_prot_hdr {
struct ice_udp_tnl_hdr tnl_hdr;
struct ice_nvgre_hdr nvgre_hdr;
struct ice_udp_gtp_hdr gtp_hdr;
+ struct ice_pppoe_hdr pppoe_hdr;
};
/* This is mapping table entry that maps every word within a given protocol
diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c b/drivers/net/ethernet/intel/ice/ice_switch.c
index 2d1274774987..4a6a8334a0e0 100644
--- a/drivers/net/ethernet/intel/ice/ice_switch.c
+++ b/drivers/net/ethernet/intel/ice/ice_switch.c
@@ -41,6 +41,7 @@ enum {
ICE_PKT_INNER_UDP = BIT(7),
ICE_PKT_GTP_NOPAY = BIT(8),
ICE_PKT_KMALLOC = BIT(9),
+ ICE_PKT_PPPOE = BIT(10),
};
struct ice_dummy_pkt_offsets {
@@ -1109,6 +1110,154 @@ ICE_DECLARE_PKT_TEMPLATE(ipv6_gtp) = {
0x00, 0x00,
};
+ICE_DECLARE_PKT_OFFSETS(pppoe_ipv4_tcp) = {
+ { ICE_MAC_OFOS, 0 },
+ { ICE_ETYPE_OL, 12 },
+ { ICE_PPPOE, 14 },
+ { ICE_IPV4_OFOS, 22 },
+ { ICE_TCP_IL, 42 },
+ { ICE_PROTOCOL_LAST, 0 },
+};
+
+ICE_DECLARE_PKT_TEMPLATE(pppoe_ipv4_tcp) = {
+ 0x00, 0x00, 0x00, 0x00, /* ICE_MAC_OFOS 0 */
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ 0x88, 0x64, /* ICE_ETYPE_OL 12 */
+
+ 0x11, 0x00, 0x00, 0x00, /* ICE_PPPOE 14 */
+ 0x00, 0x16,
+
+ 0x00, 0x21, /* PPP Link Layer 20 */
+
+ 0x45, 0x00, 0x00, 0x28, /* ICE_IPV4_OFOS 22 */
+ 0x00, 0x01, 0x00, 0x00,
+ 0x00, 0x06, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ 0x00, 0x00, 0x00, 0x00, /* ICE_TCP_IL 42 */
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x50, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ 0x00, 0x00, /* 2 bytes for 4 bytes alignment */
+};
+
+ICE_DECLARE_PKT_OFFSETS(pppoe_ipv4_udp) = {
+ { ICE_MAC_OFOS, 0 },
+ { ICE_ETYPE_OL, 12 },
+ { ICE_PPPOE, 14 },
+ { ICE_IPV4_OFOS, 22 },
+ { ICE_UDP_ILOS, 42 },
+ { ICE_PROTOCOL_LAST, 0 },
+};
+
+ICE_DECLARE_PKT_TEMPLATE(pppoe_ipv4_udp) = {
+ 0x00, 0x00, 0x00, 0x00, /* ICE_MAC_OFOS 0 */
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ 0x88, 0x64, /* ICE_ETYPE_OL 12 */
+
+ 0x11, 0x00, 0x00, 0x00, /* ICE_PPPOE 14 */
+ 0x00, 0x16,
+
+ 0x00, 0x21, /* PPP Link Layer 20 */
+
+ 0x45, 0x00, 0x00, 0x1c, /* ICE_IPV4_OFOS 22 */
+ 0x00, 0x01, 0x00, 0x00,
+ 0x00, 0x11, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ 0x00, 0x00, 0x00, 0x00, /* ICE_UDP_ILOS 42 */
+ 0x00, 0x08, 0x00, 0x00,
+
+ 0x00, 0x00, /* 2 bytes for 4 bytes alignment */
+};
+
+ICE_DECLARE_PKT_OFFSETS(pppoe_ipv6_tcp) = {
+ { ICE_MAC_OFOS, 0 },
+ { ICE_ETYPE_OL, 12 },
+ { ICE_PPPOE, 14 },
+ { ICE_IPV6_OFOS, 22 },
+ { ICE_TCP_IL, 62 },
+ { ICE_PROTOCOL_LAST, 0 },
+};
+
+ICE_DECLARE_PKT_TEMPLATE(pppoe_ipv6_tcp) = {
+ 0x00, 0x00, 0x00, 0x00, /* ICE_MAC_OFOS 0 */
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ 0x88, 0x64, /* ICE_ETYPE_OL 12 */
+
+ 0x11, 0x00, 0x00, 0x00, /* ICE_PPPOE 14 */
+ 0x00, 0x2a,
+
+ 0x00, 0x57, /* PPP Link Layer 20 */
+
+ 0x60, 0x00, 0x00, 0x00, /* ICE_IPV6_OFOS 22 */
+ 0x00, 0x14, 0x06, 0x00, /* Next header is TCP */
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ 0x00, 0x00, 0x00, 0x00, /* ICE_TCP_IL 62 */
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x50, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ 0x00, 0x00, /* 2 bytes for 4 bytes alignment */
+};
+
+ICE_DECLARE_PKT_OFFSETS(pppoe_ipv6_udp) = {
+ { ICE_MAC_OFOS, 0 },
+ { ICE_ETYPE_OL, 12 },
+ { ICE_PPPOE, 14 },
+ { ICE_IPV6_OFOS, 22 },
+ { ICE_UDP_ILOS, 62 },
+ { ICE_PROTOCOL_LAST, 0 },
+};
+
+ICE_DECLARE_PKT_TEMPLATE(pppoe_ipv6_udp) = {
+ 0x00, 0x00, 0x00, 0x00, /* ICE_MAC_OFOS 0 */
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ 0x88, 0x64, /* ICE_ETYPE_OL 12 */
+
+ 0x11, 0x00, 0x00, 0x00, /* ICE_PPPOE 14 */
+ 0x00, 0x2a,
+
+ 0x00, 0x57, /* PPP Link Layer 20 */
+
+ 0x60, 0x00, 0x00, 0x00, /* ICE_IPV6_OFOS 22 */
+ 0x00, 0x08, 0x11, 0x00, /* Next header UDP*/
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ 0x00, 0x00, 0x00, 0x00, /* ICE_UDP_ILOS 62 */
+ 0x00, 0x08, 0x00, 0x00,
+
+ 0x00, 0x00, /* 2 bytes for 4 bytes alignment */
+};
+
static const struct ice_dummy_pkt_profile ice_dummy_pkt_profiles[] = {
ICE_PKT_PROFILE(ipv6_gtp, ICE_PKT_TUN_GTPU | ICE_PKT_OUTER_IPV6 |
ICE_PKT_GTP_NOPAY),
@@ -1135,6 +1284,11 @@ static const struct ice_dummy_pkt_profile ice_dummy_pkt_profiles[] = {
ICE_PKT_PROFILE(ipv4_gtpu_ipv4_tcp, ICE_PKT_TUN_GTPU),
ICE_PKT_PROFILE(ipv6_gtp, ICE_PKT_TUN_GTPC | ICE_PKT_OUTER_IPV6),
ICE_PKT_PROFILE(ipv4_gtpu_ipv4, ICE_PKT_TUN_GTPC),
+ ICE_PKT_PROFILE(pppoe_ipv6_udp, ICE_PKT_PPPOE | ICE_PKT_OUTER_IPV6 |
+ ICE_PKT_INNER_UDP),
+ ICE_PKT_PROFILE(pppoe_ipv6_tcp, ICE_PKT_PPPOE | ICE_PKT_OUTER_IPV6),
+ ICE_PKT_PROFILE(pppoe_ipv4_udp, ICE_PKT_PPPOE | ICE_PKT_INNER_UDP),
+ ICE_PKT_PROFILE(pppoe_ipv4_tcp, ICE_PKT_PPPOE),
ICE_PKT_PROFILE(gre_ipv6_tcp, ICE_PKT_TUN_NVGRE | ICE_PKT_INNER_IPV6 |
ICE_PKT_INNER_TCP),
ICE_PKT_PROFILE(gre_tcp, ICE_PKT_TUN_NVGRE | ICE_PKT_INNER_TCP),
@@ -4480,6 +4634,7 @@ static const struct ice_prot_ext_tbl_entry ice_prot_ext[ICE_PROTOCOL_LAST] = {
{ ICE_NVGRE, { 0, 2, 4, 6 } },
{ ICE_GTP, { 8, 10, 12, 14, 16, 18, 20, 22 } },
{ ICE_GTP_NO_PAY, { 8, 10, 12, 14 } },
+ { ICE_PPPOE, { 0, 2, 4, 6 } },
{ ICE_VLAN_EX, { 2, 0 } },
{ ICE_VLAN_IN, { 2, 0 } },
};
@@ -4502,6 +4657,7 @@ static struct ice_protocol_entry ice_prot_id_tbl[ICE_PROTOCOL_LAST] = {
{ ICE_NVGRE, ICE_GRE_OF_HW },
{ ICE_GTP, ICE_UDP_OF_HW },
{ ICE_GTP_NO_PAY, ICE_UDP_ILOS_HW },
+ { ICE_PPPOE, ICE_PPPOE_HW },
{ ICE_VLAN_EX, ICE_VLAN_OF_HW },
{ ICE_VLAN_IN, ICE_VLAN_OL_HW },
};
@@ -5580,6 +5736,12 @@ ice_find_dummy_packet(struct ice_adv_lkup_elem *lkups, u16 lkups_cnt,
match |= ICE_PKT_INNER_IPV6;
else if (lkups[i].type == ICE_GTP_NO_PAY)
match |= ICE_PKT_GTP_NOPAY;
+ else if (lkups[i].type == ICE_PPPOE) {
+ match |= ICE_PKT_PPPOE;
+ if (lkups[i].h_u.pppoe_hdr.ppp_prot_id ==
+ htons(PPP_IPV6))
+ match |= ICE_PKT_OUTER_IPV6;
+ }
}
while (ret->match && (match & ret->match) != ret->match)
@@ -5677,6 +5839,9 @@ ice_fill_adv_dummy_packet(struct ice_adv_lkup_elem *lkups, u16 lkups_cnt,
case ICE_GTP:
len = sizeof(struct ice_udp_gtp_hdr);
break;
+ case ICE_PPPOE:
+ len = sizeof(struct ice_pppoe_hdr);
+ break;
default:
return -EINVAL;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_tc_lib.c b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
index 14795157846b..a298862857a8 100644
--- a/drivers/net/ethernet/intel/ice/ice_tc_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
@@ -54,6 +54,11 @@ ice_tc_count_lkups(u32 flags, struct ice_tc_flower_lyr_2_4_hdrs *headers,
if (flags & ICE_TC_FLWR_FIELD_CVLAN)
lkups_cnt++;
+ /* are PPPoE options specified? */
+ if (flags & (ICE_TC_FLWR_FIELD_PPPOE_SESSID |
+ ICE_TC_FLWR_FIELD_PPP_PROTO))
+ lkups_cnt++;
+
/* are IPv[4|6] fields specified? */
if (flags & (ICE_TC_FLWR_FIELD_DEST_IPV4 | ICE_TC_FLWR_FIELD_SRC_IPV4 |
ICE_TC_FLWR_FIELD_DEST_IPV6 | ICE_TC_FLWR_FIELD_SRC_IPV6))
@@ -350,6 +355,28 @@ ice_tc_fill_rules(struct ice_hw *hw, u32 flags,
i++;
}
+ if (flags & (ICE_TC_FLWR_FIELD_PPPOE_SESSID |
+ ICE_TC_FLWR_FIELD_PPP_PROTO)) {
+ struct ice_pppoe_hdr *vals, *masks;
+
+ vals = &list[i].h_u.pppoe_hdr;
+ masks = &list[i].m_u.pppoe_hdr;
+
+ list[i].type = ICE_PPPOE;
+
+ if (flags & ICE_TC_FLWR_FIELD_PPPOE_SESSID) {
+ vals->session_id = headers->pppoe_hdr.session_id;
+ masks->session_id = cpu_to_be16(0xFFFF);
+ }
+
+ if (flags & ICE_TC_FLWR_FIELD_PPP_PROTO) {
+ vals->ppp_prot_id = headers->pppoe_hdr.ppp_proto;
+ masks->ppp_prot_id = cpu_to_be16(0xFFFF);
+ }
+
+ i++;
+ }
+
/* copy L3 (IPv[4|6]: src, dest) address */
if (flags & (ICE_TC_FLWR_FIELD_DEST_IPV4 |
ICE_TC_FLWR_FIELD_SRC_IPV4)) {
@@ -693,6 +720,31 @@ ice_add_tc_flower_adv_fltr(struct ice_vsi *vsi,
return ret;
}
+/**
+ * ice_tc_set_pppoe - Parse PPPoE fields from TC flower filter
+ * @match: Pointer to flow match structure
+ * @fltr: Pointer to filter structure
+ * @headers: Pointer to outer header fields
+ * @returns PPP protocol used in filter (ppp_ses or ppp_disc)
+ */
+static u16
+ice_tc_set_pppoe(struct flow_match_pppoe *match,
+ struct ice_tc_flower_fltr *fltr,
+ struct ice_tc_flower_lyr_2_4_hdrs *headers)
+{
+ if (match->mask->session_id) {
+ fltr->flags |= ICE_TC_FLWR_FIELD_PPPOE_SESSID;
+ headers->pppoe_hdr.session_id = match->key->session_id;
+ }
+
+ if (match->mask->ppp_proto) {
+ fltr->flags |= ICE_TC_FLWR_FIELD_PPP_PROTO;
+ headers->pppoe_hdr.ppp_proto = match->key->ppp_proto;
+ }
+
+ return be16_to_cpu(match->key->type);
+}
+
/**
* ice_tc_set_ipv4 - Parse IPv4 addresses from TC flower filter
* @match: Pointer to flow match structure
@@ -988,7 +1040,8 @@ ice_parse_cls_flower(struct net_device *filter_dev, struct ice_vsi *vsi,
BIT(FLOW_DISSECTOR_KEY_ENC_PORTS) |
BIT(FLOW_DISSECTOR_KEY_ENC_OPTS) |
BIT(FLOW_DISSECTOR_KEY_ENC_IP) |
- BIT(FLOW_DISSECTOR_KEY_PORTS))) {
+ BIT(FLOW_DISSECTOR_KEY_PORTS) |
+ BIT(FLOW_DISSECTOR_KEY_PPPOE))) {
NL_SET_ERR_MSG_MOD(fltr->extack, "Unsupported key used");
return -EOPNOTSUPP;
}
@@ -1124,6 +1177,22 @@ ice_parse_cls_flower(struct net_device *filter_dev, struct ice_vsi *vsi,
headers->cvlan_hdr.vlan_prio = match.key->vlan_priority;
}
+ if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_PPPOE)) {
+ struct flow_match_pppoe match;
+
+ flow_rule_match_pppoe(rule, &match);
+ n_proto_key = ice_tc_set_pppoe(&match, fltr, headers);
+
+ /* If ethertype equals ETH_P_PPP_SES, n_proto might be
+ * overwritten by encapsulated protocol (ppp_proto field) or set
+ * to 0. To correct this, flow_match_pppoe provides the type
+ * field, which contains the actual ethertype (ETH_P_PPP_SES).
+ */
+ headers->l2_key.n_proto = cpu_to_be16(n_proto_key);
+ headers->l2_mask.n_proto = cpu_to_be16(0xFFFF);
+ fltr->flags |= ICE_TC_FLWR_FIELD_ETH_TYPE_ID;
+ }
+
if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_CONTROL)) {
struct flow_match_control match;
diff --git a/drivers/net/ethernet/intel/ice/ice_tc_lib.h b/drivers/net/ethernet/intel/ice/ice_tc_lib.h
index 0193874cd203..91cd3d3778c7 100644
--- a/drivers/net/ethernet/intel/ice/ice_tc_lib.h
+++ b/drivers/net/ethernet/intel/ice/ice_tc_lib.h
@@ -24,6 +24,8 @@
#define ICE_TC_FLWR_FIELD_ETH_TYPE_ID BIT(17)
#define ICE_TC_FLWR_FIELD_ENC_OPTS BIT(18)
#define ICE_TC_FLWR_FIELD_CVLAN BIT(19)
+#define ICE_TC_FLWR_FIELD_PPPOE_SESSID BIT(20)
+#define ICE_TC_FLWR_FIELD_PPP_PROTO BIT(21)
#define ICE_TC_FLOWER_MASK_32 0xFFFFFFFF
@@ -44,6 +46,11 @@ struct ice_tc_vlan_hdr {
__be16 vlan_tpid;
};
+struct ice_tc_pppoe_hdr {
+ __be16 session_id;
+ __be16 ppp_proto;
+};
+
struct ice_tc_l2_hdr {
u8 dst_mac[ETH_ALEN];
u8 src_mac[ETH_ALEN];
@@ -84,6 +91,7 @@ struct ice_tc_flower_lyr_2_4_hdrs {
struct ice_tc_l2_hdr l2_mask;
struct ice_tc_vlan_hdr vlan_hdr;
struct ice_tc_vlan_hdr cvlan_hdr;
+ struct ice_tc_pppoe_hdr pppoe_hdr;
/* L3 (IPv4[6]) layer fields with their mask */
struct ice_tc_l3_hdr l3_key;
struct ice_tc_l3_hdr l3_mask;
--
2.35.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Intel-wired-lan] [RFC PATCH net-next v4 4/4] ice: Add support for PPPoE hardware offload
@ 2022-07-08 12:24 ` Marcin Szycik
0 siblings, 0 replies; 28+ messages in thread
From: Marcin Szycik @ 2022-07-08 12:24 UTC (permalink / raw)
To: netdev
Cc: simon.horman, kurt, paulb, edumazet, boris.sukholitko, jiri,
paulus, gnault, jesse.brandeburg, intel-wired-lan, kuba,
zhangkaiheb, pablo, baowen.zheng, komachi.yoshiki, jhs, mostrows,
xiyou.wangcong, pabeni, gustavoars, davem
Add support for creating PPPoE filters in switchdev mode. Add support
for parsing PPPoE and PPP-specific tc options: pppoe_sid and ppp_proto.
Example filter:
tc filter add dev $PF1 ingress protocol ppp_ses prio 1 flower pppoe_sid \
1234 ppp_proto ip skip_sw action mirred egress redirect dev $VF1_PR
Changes in iproute2 are required to use the new fields.
ICE COMMS DDP package is required to create a filter as it contains PPPoE
profiles. Added a warning message when loaded DDP package does not contain
required profiles.
Signed-off-by: Marcin Szycik <marcin.szycik@linux.intel.com>
---
v4:
* adapted code to the redefined ice_tc_pppoe_hdr (session_id is now
stored in __be16)
* removed note about missing support for vlan fields:
263957263a00 ("ice: switch: dynamically add VLAN headers to dummy packets")
is already accepted
* always add an ethtype lookup if PPP/PPPoE options are provided (to
prevent setting incorrect ethtype)
* rebase
v2: wrap one long line
drivers/net/ethernet/intel/ice/ice.h | 1 +
.../net/ethernet/intel/ice/ice_flex_pipe.c | 5 +-
.../ethernet/intel/ice/ice_protocol_type.h | 11 ++
drivers/net/ethernet/intel/ice/ice_switch.c | 165 ++++++++++++++++++
drivers/net/ethernet/intel/ice/ice_tc_lib.c | 71 +++++++-
drivers/net/ethernet/intel/ice/ice_tc_lib.h | 8 +
6 files changed, 259 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index 60453b3b8d23..0e48a930f004 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -52,6 +52,7 @@
#include <net/udp_tunnel.h>
#include <net/vxlan.h>
#include <net/gtp.h>
+#include <linux/ppp_defs.h>
#include "ice_devids.h"
#include "ice_type.h"
#include "ice_txrx.h"
diff --git a/drivers/net/ethernet/intel/ice/ice_flex_pipe.c b/drivers/net/ethernet/intel/ice/ice_flex_pipe.c
index ada5198b5b16..4b3bb19e1d06 100644
--- a/drivers/net/ethernet/intel/ice/ice_flex_pipe.c
+++ b/drivers/net/ethernet/intel/ice/ice_flex_pipe.c
@@ -1964,8 +1964,11 @@ ice_get_sw_fv_list(struct ice_hw *hw, struct ice_prot_lkup_ext *lkups,
}
}
} while (fv);
- if (list_empty(fv_list))
+ if (list_empty(fv_list)) {
+ dev_warn(ice_hw_to_dev(hw), "Required profiles not found in currently loaded DDP package");
return -EIO;
+ }
+
return 0;
err:
diff --git a/drivers/net/ethernet/intel/ice/ice_protocol_type.h b/drivers/net/ethernet/intel/ice/ice_protocol_type.h
index d4a0d089649c..560efc7654c7 100644
--- a/drivers/net/ethernet/intel/ice/ice_protocol_type.h
+++ b/drivers/net/ethernet/intel/ice/ice_protocol_type.h
@@ -43,6 +43,7 @@ enum ice_protocol_type {
ICE_NVGRE,
ICE_GTP,
ICE_GTP_NO_PAY,
+ ICE_PPPOE,
ICE_VLAN_EX,
ICE_VLAN_IN,
ICE_VXLAN_GPE,
@@ -109,6 +110,7 @@ enum ice_prot_id {
#define ICE_TCP_IL_HW 49
#define ICE_UDP_ILOS_HW 53
#define ICE_GRE_OF_HW 64
+#define ICE_PPPOE_HW 103
#define ICE_UDP_OF_HW 52 /* UDP Tunnels */
#define ICE_META_DATA_ID_HW 255 /* this is used for tunnel and VLAN type */
@@ -207,6 +209,14 @@ struct ice_udp_gtp_hdr {
u8 rsvrd;
};
+struct ice_pppoe_hdr {
+ u8 rsrvd_ver_type;
+ u8 rsrvd_code;
+ __be16 session_id;
+ __be16 length;
+ __be16 ppp_prot_id; /* control and data only */
+};
+
struct ice_nvgre_hdr {
__be16 flags;
__be16 protocol;
@@ -224,6 +234,7 @@ union ice_prot_hdr {
struct ice_udp_tnl_hdr tnl_hdr;
struct ice_nvgre_hdr nvgre_hdr;
struct ice_udp_gtp_hdr gtp_hdr;
+ struct ice_pppoe_hdr pppoe_hdr;
};
/* This is mapping table entry that maps every word within a given protocol
diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c b/drivers/net/ethernet/intel/ice/ice_switch.c
index 2d1274774987..4a6a8334a0e0 100644
--- a/drivers/net/ethernet/intel/ice/ice_switch.c
+++ b/drivers/net/ethernet/intel/ice/ice_switch.c
@@ -41,6 +41,7 @@ enum {
ICE_PKT_INNER_UDP = BIT(7),
ICE_PKT_GTP_NOPAY = BIT(8),
ICE_PKT_KMALLOC = BIT(9),
+ ICE_PKT_PPPOE = BIT(10),
};
struct ice_dummy_pkt_offsets {
@@ -1109,6 +1110,154 @@ ICE_DECLARE_PKT_TEMPLATE(ipv6_gtp) = {
0x00, 0x00,
};
+ICE_DECLARE_PKT_OFFSETS(pppoe_ipv4_tcp) = {
+ { ICE_MAC_OFOS, 0 },
+ { ICE_ETYPE_OL, 12 },
+ { ICE_PPPOE, 14 },
+ { ICE_IPV4_OFOS, 22 },
+ { ICE_TCP_IL, 42 },
+ { ICE_PROTOCOL_LAST, 0 },
+};
+
+ICE_DECLARE_PKT_TEMPLATE(pppoe_ipv4_tcp) = {
+ 0x00, 0x00, 0x00, 0x00, /* ICE_MAC_OFOS 0 */
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ 0x88, 0x64, /* ICE_ETYPE_OL 12 */
+
+ 0x11, 0x00, 0x00, 0x00, /* ICE_PPPOE 14 */
+ 0x00, 0x16,
+
+ 0x00, 0x21, /* PPP Link Layer 20 */
+
+ 0x45, 0x00, 0x00, 0x28, /* ICE_IPV4_OFOS 22 */
+ 0x00, 0x01, 0x00, 0x00,
+ 0x00, 0x06, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ 0x00, 0x00, 0x00, 0x00, /* ICE_TCP_IL 42 */
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x50, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ 0x00, 0x00, /* 2 bytes for 4 bytes alignment */
+};
+
+ICE_DECLARE_PKT_OFFSETS(pppoe_ipv4_udp) = {
+ { ICE_MAC_OFOS, 0 },
+ { ICE_ETYPE_OL, 12 },
+ { ICE_PPPOE, 14 },
+ { ICE_IPV4_OFOS, 22 },
+ { ICE_UDP_ILOS, 42 },
+ { ICE_PROTOCOL_LAST, 0 },
+};
+
+ICE_DECLARE_PKT_TEMPLATE(pppoe_ipv4_udp) = {
+ 0x00, 0x00, 0x00, 0x00, /* ICE_MAC_OFOS 0 */
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ 0x88, 0x64, /* ICE_ETYPE_OL 12 */
+
+ 0x11, 0x00, 0x00, 0x00, /* ICE_PPPOE 14 */
+ 0x00, 0x16,
+
+ 0x00, 0x21, /* PPP Link Layer 20 */
+
+ 0x45, 0x00, 0x00, 0x1c, /* ICE_IPV4_OFOS 22 */
+ 0x00, 0x01, 0x00, 0x00,
+ 0x00, 0x11, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ 0x00, 0x00, 0x00, 0x00, /* ICE_UDP_ILOS 42 */
+ 0x00, 0x08, 0x00, 0x00,
+
+ 0x00, 0x00, /* 2 bytes for 4 bytes alignment */
+};
+
+ICE_DECLARE_PKT_OFFSETS(pppoe_ipv6_tcp) = {
+ { ICE_MAC_OFOS, 0 },
+ { ICE_ETYPE_OL, 12 },
+ { ICE_PPPOE, 14 },
+ { ICE_IPV6_OFOS, 22 },
+ { ICE_TCP_IL, 62 },
+ { ICE_PROTOCOL_LAST, 0 },
+};
+
+ICE_DECLARE_PKT_TEMPLATE(pppoe_ipv6_tcp) = {
+ 0x00, 0x00, 0x00, 0x00, /* ICE_MAC_OFOS 0 */
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ 0x88, 0x64, /* ICE_ETYPE_OL 12 */
+
+ 0x11, 0x00, 0x00, 0x00, /* ICE_PPPOE 14 */
+ 0x00, 0x2a,
+
+ 0x00, 0x57, /* PPP Link Layer 20 */
+
+ 0x60, 0x00, 0x00, 0x00, /* ICE_IPV6_OFOS 22 */
+ 0x00, 0x14, 0x06, 0x00, /* Next header is TCP */
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ 0x00, 0x00, 0x00, 0x00, /* ICE_TCP_IL 62 */
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x50, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ 0x00, 0x00, /* 2 bytes for 4 bytes alignment */
+};
+
+ICE_DECLARE_PKT_OFFSETS(pppoe_ipv6_udp) = {
+ { ICE_MAC_OFOS, 0 },
+ { ICE_ETYPE_OL, 12 },
+ { ICE_PPPOE, 14 },
+ { ICE_IPV6_OFOS, 22 },
+ { ICE_UDP_ILOS, 62 },
+ { ICE_PROTOCOL_LAST, 0 },
+};
+
+ICE_DECLARE_PKT_TEMPLATE(pppoe_ipv6_udp) = {
+ 0x00, 0x00, 0x00, 0x00, /* ICE_MAC_OFOS 0 */
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ 0x88, 0x64, /* ICE_ETYPE_OL 12 */
+
+ 0x11, 0x00, 0x00, 0x00, /* ICE_PPPOE 14 */
+ 0x00, 0x2a,
+
+ 0x00, 0x57, /* PPP Link Layer 20 */
+
+ 0x60, 0x00, 0x00, 0x00, /* ICE_IPV6_OFOS 22 */
+ 0x00, 0x08, 0x11, 0x00, /* Next header UDP*/
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ 0x00, 0x00, 0x00, 0x00, /* ICE_UDP_ILOS 62 */
+ 0x00, 0x08, 0x00, 0x00,
+
+ 0x00, 0x00, /* 2 bytes for 4 bytes alignment */
+};
+
static const struct ice_dummy_pkt_profile ice_dummy_pkt_profiles[] = {
ICE_PKT_PROFILE(ipv6_gtp, ICE_PKT_TUN_GTPU | ICE_PKT_OUTER_IPV6 |
ICE_PKT_GTP_NOPAY),
@@ -1135,6 +1284,11 @@ static const struct ice_dummy_pkt_profile ice_dummy_pkt_profiles[] = {
ICE_PKT_PROFILE(ipv4_gtpu_ipv4_tcp, ICE_PKT_TUN_GTPU),
ICE_PKT_PROFILE(ipv6_gtp, ICE_PKT_TUN_GTPC | ICE_PKT_OUTER_IPV6),
ICE_PKT_PROFILE(ipv4_gtpu_ipv4, ICE_PKT_TUN_GTPC),
+ ICE_PKT_PROFILE(pppoe_ipv6_udp, ICE_PKT_PPPOE | ICE_PKT_OUTER_IPV6 |
+ ICE_PKT_INNER_UDP),
+ ICE_PKT_PROFILE(pppoe_ipv6_tcp, ICE_PKT_PPPOE | ICE_PKT_OUTER_IPV6),
+ ICE_PKT_PROFILE(pppoe_ipv4_udp, ICE_PKT_PPPOE | ICE_PKT_INNER_UDP),
+ ICE_PKT_PROFILE(pppoe_ipv4_tcp, ICE_PKT_PPPOE),
ICE_PKT_PROFILE(gre_ipv6_tcp, ICE_PKT_TUN_NVGRE | ICE_PKT_INNER_IPV6 |
ICE_PKT_INNER_TCP),
ICE_PKT_PROFILE(gre_tcp, ICE_PKT_TUN_NVGRE | ICE_PKT_INNER_TCP),
@@ -4480,6 +4634,7 @@ static const struct ice_prot_ext_tbl_entry ice_prot_ext[ICE_PROTOCOL_LAST] = {
{ ICE_NVGRE, { 0, 2, 4, 6 } },
{ ICE_GTP, { 8, 10, 12, 14, 16, 18, 20, 22 } },
{ ICE_GTP_NO_PAY, { 8, 10, 12, 14 } },
+ { ICE_PPPOE, { 0, 2, 4, 6 } },
{ ICE_VLAN_EX, { 2, 0 } },
{ ICE_VLAN_IN, { 2, 0 } },
};
@@ -4502,6 +4657,7 @@ static struct ice_protocol_entry ice_prot_id_tbl[ICE_PROTOCOL_LAST] = {
{ ICE_NVGRE, ICE_GRE_OF_HW },
{ ICE_GTP, ICE_UDP_OF_HW },
{ ICE_GTP_NO_PAY, ICE_UDP_ILOS_HW },
+ { ICE_PPPOE, ICE_PPPOE_HW },
{ ICE_VLAN_EX, ICE_VLAN_OF_HW },
{ ICE_VLAN_IN, ICE_VLAN_OL_HW },
};
@@ -5580,6 +5736,12 @@ ice_find_dummy_packet(struct ice_adv_lkup_elem *lkups, u16 lkups_cnt,
match |= ICE_PKT_INNER_IPV6;
else if (lkups[i].type == ICE_GTP_NO_PAY)
match |= ICE_PKT_GTP_NOPAY;
+ else if (lkups[i].type == ICE_PPPOE) {
+ match |= ICE_PKT_PPPOE;
+ if (lkups[i].h_u.pppoe_hdr.ppp_prot_id ==
+ htons(PPP_IPV6))
+ match |= ICE_PKT_OUTER_IPV6;
+ }
}
while (ret->match && (match & ret->match) != ret->match)
@@ -5677,6 +5839,9 @@ ice_fill_adv_dummy_packet(struct ice_adv_lkup_elem *lkups, u16 lkups_cnt,
case ICE_GTP:
len = sizeof(struct ice_udp_gtp_hdr);
break;
+ case ICE_PPPOE:
+ len = sizeof(struct ice_pppoe_hdr);
+ break;
default:
return -EINVAL;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_tc_lib.c b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
index 14795157846b..a298862857a8 100644
--- a/drivers/net/ethernet/intel/ice/ice_tc_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
@@ -54,6 +54,11 @@ ice_tc_count_lkups(u32 flags, struct ice_tc_flower_lyr_2_4_hdrs *headers,
if (flags & ICE_TC_FLWR_FIELD_CVLAN)
lkups_cnt++;
+ /* are PPPoE options specified? */
+ if (flags & (ICE_TC_FLWR_FIELD_PPPOE_SESSID |
+ ICE_TC_FLWR_FIELD_PPP_PROTO))
+ lkups_cnt++;
+
/* are IPv[4|6] fields specified? */
if (flags & (ICE_TC_FLWR_FIELD_DEST_IPV4 | ICE_TC_FLWR_FIELD_SRC_IPV4 |
ICE_TC_FLWR_FIELD_DEST_IPV6 | ICE_TC_FLWR_FIELD_SRC_IPV6))
@@ -350,6 +355,28 @@ ice_tc_fill_rules(struct ice_hw *hw, u32 flags,
i++;
}
+ if (flags & (ICE_TC_FLWR_FIELD_PPPOE_SESSID |
+ ICE_TC_FLWR_FIELD_PPP_PROTO)) {
+ struct ice_pppoe_hdr *vals, *masks;
+
+ vals = &list[i].h_u.pppoe_hdr;
+ masks = &list[i].m_u.pppoe_hdr;
+
+ list[i].type = ICE_PPPOE;
+
+ if (flags & ICE_TC_FLWR_FIELD_PPPOE_SESSID) {
+ vals->session_id = headers->pppoe_hdr.session_id;
+ masks->session_id = cpu_to_be16(0xFFFF);
+ }
+
+ if (flags & ICE_TC_FLWR_FIELD_PPP_PROTO) {
+ vals->ppp_prot_id = headers->pppoe_hdr.ppp_proto;
+ masks->ppp_prot_id = cpu_to_be16(0xFFFF);
+ }
+
+ i++;
+ }
+
/* copy L3 (IPv[4|6]: src, dest) address */
if (flags & (ICE_TC_FLWR_FIELD_DEST_IPV4 |
ICE_TC_FLWR_FIELD_SRC_IPV4)) {
@@ -693,6 +720,31 @@ ice_add_tc_flower_adv_fltr(struct ice_vsi *vsi,
return ret;
}
+/**
+ * ice_tc_set_pppoe - Parse PPPoE fields from TC flower filter
+ * @match: Pointer to flow match structure
+ * @fltr: Pointer to filter structure
+ * @headers: Pointer to outer header fields
+ * @returns PPP protocol used in filter (ppp_ses or ppp_disc)
+ */
+static u16
+ice_tc_set_pppoe(struct flow_match_pppoe *match,
+ struct ice_tc_flower_fltr *fltr,
+ struct ice_tc_flower_lyr_2_4_hdrs *headers)
+{
+ if (match->mask->session_id) {
+ fltr->flags |= ICE_TC_FLWR_FIELD_PPPOE_SESSID;
+ headers->pppoe_hdr.session_id = match->key->session_id;
+ }
+
+ if (match->mask->ppp_proto) {
+ fltr->flags |= ICE_TC_FLWR_FIELD_PPP_PROTO;
+ headers->pppoe_hdr.ppp_proto = match->key->ppp_proto;
+ }
+
+ return be16_to_cpu(match->key->type);
+}
+
/**
* ice_tc_set_ipv4 - Parse IPv4 addresses from TC flower filter
* @match: Pointer to flow match structure
@@ -988,7 +1040,8 @@ ice_parse_cls_flower(struct net_device *filter_dev, struct ice_vsi *vsi,
BIT(FLOW_DISSECTOR_KEY_ENC_PORTS) |
BIT(FLOW_DISSECTOR_KEY_ENC_OPTS) |
BIT(FLOW_DISSECTOR_KEY_ENC_IP) |
- BIT(FLOW_DISSECTOR_KEY_PORTS))) {
+ BIT(FLOW_DISSECTOR_KEY_PORTS) |
+ BIT(FLOW_DISSECTOR_KEY_PPPOE))) {
NL_SET_ERR_MSG_MOD(fltr->extack, "Unsupported key used");
return -EOPNOTSUPP;
}
@@ -1124,6 +1177,22 @@ ice_parse_cls_flower(struct net_device *filter_dev, struct ice_vsi *vsi,
headers->cvlan_hdr.vlan_prio = match.key->vlan_priority;
}
+ if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_PPPOE)) {
+ struct flow_match_pppoe match;
+
+ flow_rule_match_pppoe(rule, &match);
+ n_proto_key = ice_tc_set_pppoe(&match, fltr, headers);
+
+ /* If ethertype equals ETH_P_PPP_SES, n_proto might be
+ * overwritten by encapsulated protocol (ppp_proto field) or set
+ * to 0. To correct this, flow_match_pppoe provides the type
+ * field, which contains the actual ethertype (ETH_P_PPP_SES).
+ */
+ headers->l2_key.n_proto = cpu_to_be16(n_proto_key);
+ headers->l2_mask.n_proto = cpu_to_be16(0xFFFF);
+ fltr->flags |= ICE_TC_FLWR_FIELD_ETH_TYPE_ID;
+ }
+
if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_CONTROL)) {
struct flow_match_control match;
diff --git a/drivers/net/ethernet/intel/ice/ice_tc_lib.h b/drivers/net/ethernet/intel/ice/ice_tc_lib.h
index 0193874cd203..91cd3d3778c7 100644
--- a/drivers/net/ethernet/intel/ice/ice_tc_lib.h
+++ b/drivers/net/ethernet/intel/ice/ice_tc_lib.h
@@ -24,6 +24,8 @@
#define ICE_TC_FLWR_FIELD_ETH_TYPE_ID BIT(17)
#define ICE_TC_FLWR_FIELD_ENC_OPTS BIT(18)
#define ICE_TC_FLWR_FIELD_CVLAN BIT(19)
+#define ICE_TC_FLWR_FIELD_PPPOE_SESSID BIT(20)
+#define ICE_TC_FLWR_FIELD_PPP_PROTO BIT(21)
#define ICE_TC_FLOWER_MASK_32 0xFFFFFFFF
@@ -44,6 +46,11 @@ struct ice_tc_vlan_hdr {
__be16 vlan_tpid;
};
+struct ice_tc_pppoe_hdr {
+ __be16 session_id;
+ __be16 ppp_proto;
+};
+
struct ice_tc_l2_hdr {
u8 dst_mac[ETH_ALEN];
u8 src_mac[ETH_ALEN];
@@ -84,6 +91,7 @@ struct ice_tc_flower_lyr_2_4_hdrs {
struct ice_tc_l2_hdr l2_mask;
struct ice_tc_vlan_hdr vlan_hdr;
struct ice_tc_vlan_hdr cvlan_hdr;
+ struct ice_tc_pppoe_hdr pppoe_hdr;
/* L3 (IPv4[6]) layer fields with their mask */
struct ice_tc_l3_hdr l3_key;
struct ice_tc_l3_hdr l3_mask;
--
2.35.1
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [RFC PATCH net-next v4 1/4] flow_dissector: Add PPPoE dissectors
2022-07-08 12:24 ` [Intel-wired-lan] " Marcin Szycik
@ 2022-07-08 19:05 ` Guillaume Nault
-1 siblings, 0 replies; 28+ messages in thread
From: Guillaume Nault @ 2022-07-08 19:05 UTC (permalink / raw)
To: Marcin Szycik
Cc: netdev, anthony.l.nguyen, davem, xiyou.wangcong,
jesse.brandeburg, gustavoars, baowen.zheng, boris.sukholitko,
edumazet, kuba, jhs, jiri, kurt, pablo, pabeni, paulb,
simon.horman, komachi.yoshiki, zhangkaiheb, intel-wired-lan,
michal.swiatkowski, wojciech.drewek, alexandr.lobakin, mostrows,
paulus
On Fri, Jul 08, 2022 at 02:24:18PM +0200, Marcin Szycik wrote:
> From: Wojciech Drewek <wojciech.drewek@intel.com>
>
> Allow to dissect PPPoE specific fields which are:
> - session ID (16 bits)
> - ppp protocol (16 bits)
> - type (16 bits) - this is PPPoE ethertype, for now only
> ETH_P_PPP_SES is supported, possible ETH_P_PPP_DISC
> in the future
>
> 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>
> ---
> v4:
> * pppoe header validation
> * added MPLS dissection
> * added support for compressed ppp protocol field
> * flow_dissector_key_pppoe::session_id stored in __be16
> * new field: flow_dissector_key_pppoe::type
> v3: revert byte order changes in is_ppp_proto_supported from
> previous version
> v2: ntohs instead of htons in is_ppp_proto_supported
>
> include/net/flow_dissector.h | 13 ++++++
> net/core/flow_dissector.c | 84 +++++++++++++++++++++++++++++++++---
> 2 files changed, 90 insertions(+), 7 deletions(-)
>
> diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
> index a4c6057c7097..af0d429b9a26 100644
> --- a/include/net/flow_dissector.h
> +++ b/include/net/flow_dissector.h
> @@ -261,6 +261,18 @@ 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
> + * @type: pppoe eth type
> + */
> +struct flow_dissector_key_pppoe {
> + __be16 session_id;
> + __be16 ppp_proto;
> + __be16 type;
I don't understand the need for the new 'type' field.
> +};
> +
> 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 +303,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..3a90219d2354 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -895,6 +895,42 @@ bool bpf_flow_dissect(struct bpf_prog *prog, struct bpf_flow_dissector *ctx,
> return result == BPF_OK;
> }
>
> +/**
> + * is_ppp_proto_supported - checks if inner PPP protocol should be dissected
> + * @proto: protocol type (PPP proto field)
> + */
> +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_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;
> + }
> +}
> +
> +static bool is_pppoe_ses_hdr_valid(struct pppoe_hdr hdr)
> +{
> + return hdr.ver == 1 && hdr.type == 1 && hdr.code == 0;
> +}
> +
> /**
> * __skb_flow_dissect - extract the flow_keys struct and return it
> * @net: associated network namespace, derived from @skb if NULL
> @@ -1214,26 +1250,60 @@ bool __skb_flow_dissect(const struct net *net,
> struct pppoe_hdr hdr;
> __be16 proto;
> } *hdr, _hdr;
> + __be16 ppp_proto;
> +
> hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr);
> if (!hdr) {
> fdret = FLOW_DISSECT_RET_OUT_BAD;
> break;
> }
>
> - nhoff += PPPOE_SES_HLEN;
> - switch (hdr->proto) {
> - case htons(PPP_IP):
> + if (!is_pppoe_ses_hdr_valid(hdr->hdr)) {
> + fdret = FLOW_DISSECT_RET_OUT_BAD;
> + break;
> + }
> +
> + /* least significant bit of the first byte
> + * indicates if protocol field was compressed
> + */
> + if (hdr->proto & 1) {
> + ppp_proto = hdr->proto << 8;
This is little endian specific code. We can't make such assumptions.
> + nhoff += PPPOE_SES_HLEN - 1;
> + } else {
> + ppp_proto = hdr->proto;
> + nhoff += PPPOE_SES_HLEN;
> + }
> +
> + if (ppp_proto == htons(PPP_IP)) {
> proto = htons(ETH_P_IP);
> fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
> - break;
> - case htons(PPP_IPV6):
> + } else if (ppp_proto == htons(PPP_IPV6)) {
> proto = htons(ETH_P_IPV6);
> fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
> - break;
> - default:
> + } else if (ppp_proto == htons(PPP_MPLS_UC)) {
> + proto = htons(ETH_P_MPLS_UC);
> + fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
> + } else if (ppp_proto == htons(PPP_MPLS_MC)) {
> + proto = htons(ETH_P_MPLS_MC);
> + fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
> + } else if (is_ppp_proto_supported(ppp_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 = hdr->hdr.sid;
> + key_pppoe->ppp_proto = ppp_proto;
> + key_pppoe->type = htons(ETH_P_PPP_SES);
> + }
> break;
> }
> case htons(ETH_P_TIPC): {
> --
> 2.35.1
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Intel-wired-lan] [RFC PATCH net-next v4 1/4] flow_dissector: Add PPPoE dissectors
@ 2022-07-08 19:05 ` Guillaume Nault
0 siblings, 0 replies; 28+ messages in thread
From: Guillaume Nault @ 2022-07-08 19:05 UTC (permalink / raw)
To: Marcin Szycik
Cc: simon.horman, kurt, paulb, edumazet, boris.sukholitko, jiri,
paulus, jesse.brandeburg, intel-wired-lan, kuba, zhangkaiheb,
pablo, baowen.zheng, komachi.yoshiki, jhs, mostrows,
xiyou.wangcong, pabeni, netdev, gustavoars, davem
On Fri, Jul 08, 2022 at 02:24:18PM +0200, Marcin Szycik wrote:
> From: Wojciech Drewek <wojciech.drewek@intel.com>
>
> Allow to dissect PPPoE specific fields which are:
> - session ID (16 bits)
> - ppp protocol (16 bits)
> - type (16 bits) - this is PPPoE ethertype, for now only
> ETH_P_PPP_SES is supported, possible ETH_P_PPP_DISC
> in the future
>
> 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>
> ---
> v4:
> * pppoe header validation
> * added MPLS dissection
> * added support for compressed ppp protocol field
> * flow_dissector_key_pppoe::session_id stored in __be16
> * new field: flow_dissector_key_pppoe::type
> v3: revert byte order changes in is_ppp_proto_supported from
> previous version
> v2: ntohs instead of htons in is_ppp_proto_supported
>
> include/net/flow_dissector.h | 13 ++++++
> net/core/flow_dissector.c | 84 +++++++++++++++++++++++++++++++++---
> 2 files changed, 90 insertions(+), 7 deletions(-)
>
> diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
> index a4c6057c7097..af0d429b9a26 100644
> --- a/include/net/flow_dissector.h
> +++ b/include/net/flow_dissector.h
> @@ -261,6 +261,18 @@ 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
> + * @type: pppoe eth type
> + */
> +struct flow_dissector_key_pppoe {
> + __be16 session_id;
> + __be16 ppp_proto;
> + __be16 type;
I don't understand the need for the new 'type' field.
> +};
> +
> 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 +303,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..3a90219d2354 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -895,6 +895,42 @@ bool bpf_flow_dissect(struct bpf_prog *prog, struct bpf_flow_dissector *ctx,
> return result == BPF_OK;
> }
>
> +/**
> + * is_ppp_proto_supported - checks if inner PPP protocol should be dissected
> + * @proto: protocol type (PPP proto field)
> + */
> +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_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;
> + }
> +}
> +
> +static bool is_pppoe_ses_hdr_valid(struct pppoe_hdr hdr)
> +{
> + return hdr.ver == 1 && hdr.type == 1 && hdr.code == 0;
> +}
> +
> /**
> * __skb_flow_dissect - extract the flow_keys struct and return it
> * @net: associated network namespace, derived from @skb if NULL
> @@ -1214,26 +1250,60 @@ bool __skb_flow_dissect(const struct net *net,
> struct pppoe_hdr hdr;
> __be16 proto;
> } *hdr, _hdr;
> + __be16 ppp_proto;
> +
> hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr);
> if (!hdr) {
> fdret = FLOW_DISSECT_RET_OUT_BAD;
> break;
> }
>
> - nhoff += PPPOE_SES_HLEN;
> - switch (hdr->proto) {
> - case htons(PPP_IP):
> + if (!is_pppoe_ses_hdr_valid(hdr->hdr)) {
> + fdret = FLOW_DISSECT_RET_OUT_BAD;
> + break;
> + }
> +
> + /* least significant bit of the first byte
> + * indicates if protocol field was compressed
> + */
> + if (hdr->proto & 1) {
> + ppp_proto = hdr->proto << 8;
This is little endian specific code. We can't make such assumptions.
> + nhoff += PPPOE_SES_HLEN - 1;
> + } else {
> + ppp_proto = hdr->proto;
> + nhoff += PPPOE_SES_HLEN;
> + }
> +
> + if (ppp_proto == htons(PPP_IP)) {
> proto = htons(ETH_P_IP);
> fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
> - break;
> - case htons(PPP_IPV6):
> + } else if (ppp_proto == htons(PPP_IPV6)) {
> proto = htons(ETH_P_IPV6);
> fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
> - break;
> - default:
> + } else if (ppp_proto == htons(PPP_MPLS_UC)) {
> + proto = htons(ETH_P_MPLS_UC);
> + fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
> + } else if (ppp_proto == htons(PPP_MPLS_MC)) {
> + proto = htons(ETH_P_MPLS_MC);
> + fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
> + } else if (is_ppp_proto_supported(ppp_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 = hdr->hdr.sid;
> + key_pppoe->ppp_proto = ppp_proto;
> + key_pppoe->type = htons(ETH_P_PPP_SES);
> + }
> break;
> }
> case htons(ETH_P_TIPC): {
> --
> 2.35.1
>
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH net-next v4 2/4] net/sched: flower: Add PPPoE filter
2022-07-08 12:24 ` [Intel-wired-lan] " Marcin Szycik
@ 2022-07-08 19:22 ` Guillaume Nault
-1 siblings, 0 replies; 28+ messages in thread
From: Guillaume Nault @ 2022-07-08 19:22 UTC (permalink / raw)
To: Marcin Szycik
Cc: netdev, anthony.l.nguyen, davem, xiyou.wangcong,
jesse.brandeburg, gustavoars, baowen.zheng, boris.sukholitko,
edumazet, kuba, jhs, jiri, kurt, pablo, pabeni, paulb,
simon.horman, komachi.yoshiki, zhangkaiheb, intel-wired-lan,
michal.swiatkowski, wojciech.drewek, alexandr.lobakin, mostrows,
paulus
On Fri, Jul 08, 2022 at 02:24:19PM +0200, Marcin Szycik wrote:
> From: Wojciech Drewek <wojciech.drewek@intel.com>
>
> Add support for PPPoE specific fields for tc-flower.
> Those fields can be provided only when protocol was set
> to ETH_P_PPP_SES. Defines, dump, load and set are being done here.
>
> Overwrite basic.n_proto only in case of PPP_IP and PPP_IPV6,
... and PPP_MPLS_UC/PPP_MPLS_MC in this new patch version.
> otherwise leave it as ETH_P_PPP_SES.
>
> Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
> ---
> v4:
> * support of MPLS inner fields
> * session_id stored in __be16
>
> include/uapi/linux/pkt_cls.h | 3 ++
> net/sched/cls_flower.c | 61 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 64 insertions(+)
>
> diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
> index 9a2ee1e39fad..c142c0f8ed8a 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, /* be16 */
> + TCA_FLOWER_KEY_PPP_PROTO, /* be16 */
> +
> __TCA_FLOWER_MAX,
> };
>
> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> index dcca70144dff..2a0ebad0e61c 100644
> --- a/net/sched/cls_flower.c
> +++ b/net/sched/cls_flower.c
> @@ -16,6 +16,7 @@
> #include <linux/in6.h>
> #include <linux/ip.h>
> #include <linux/mpls.h>
> +#include <linux/ppp_defs.h>
>
> #include <net/sch_generic.h>
> #include <net/pkt_cls.h>
> @@ -73,6 +74,7 @@ struct fl_flow_key {
> struct flow_dissector_key_ct ct;
> struct flow_dissector_key_hash hash;
> struct flow_dissector_key_num_of_vlans num_of_vlans;
> + struct flow_dissector_key_pppoe pppoe;
> } __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */
>
> struct fl_flow_mask_range {
> @@ -714,6 +716,8 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
> [TCA_FLOWER_KEY_HASH] = { .type = NLA_U32 },
> [TCA_FLOWER_KEY_HASH_MASK] = { .type = NLA_U32 },
> [TCA_FLOWER_KEY_NUM_OF_VLANS] = { .type = NLA_U8 },
> + [TCA_FLOWER_KEY_PPPOE_SID] = { .type = NLA_U16 },
> + [TCA_FLOWER_KEY_PPP_PROTO] = { .type = NLA_U16 },
>
> };
>
> @@ -1041,6 +1045,50 @@ static void fl_set_key_vlan(struct nlattr **tb,
> }
> }
>
> +static void fl_set_key_pppoe(struct nlattr **tb,
> + struct flow_dissector_key_pppoe *key_val,
> + struct flow_dissector_key_pppoe *key_mask,
> + struct fl_flow_key *key,
> + struct fl_flow_key *mask)
> +{
> + /* key_val::type must be set to ETH_P_PPP_SES
> + * because ETH_P_PPP_SES was stored in basic.n_proto
> + * which might get overwritten by ppp_proto
> + * or might be set to 0, the role of key_val::type
> + * is simmilar to vlan_key::tpid
Didn't you mean "vlan_tpid"?
> + */
> + key_val->type = htons(ETH_P_PPP_SES);
> + key_mask->type = cpu_to_be16(~0);
> +
> + if (tb[TCA_FLOWER_KEY_PPPOE_SID]) {
> + key_val->session_id =
> + nla_get_be16(tb[TCA_FLOWER_KEY_PPPOE_SID]);
> + key_mask->session_id = cpu_to_be16(~0);
> + }
> + if (tb[TCA_FLOWER_KEY_PPP_PROTO]) {
> + key_val->ppp_proto =
> + nla_get_be16(tb[TCA_FLOWER_KEY_PPP_PROTO]);
> + key_mask->ppp_proto = cpu_to_be16(~0);
> +
> + if (key_val->ppp_proto == htons(PPP_IP)) {
> + key->basic.n_proto = htons(ETH_P_IP);
> + mask->basic.n_proto = cpu_to_be16(~0);
> + } else if (key_val->ppp_proto == htons(PPP_IPV6)) {
> + key->basic.n_proto = htons(ETH_P_IPV6);
> + mask->basic.n_proto = cpu_to_be16(~0);
> + } else if (key_val->ppp_proto == htons(PPP_MPLS_UC)) {
> + key->basic.n_proto = htons(ETH_P_MPLS_UC);
> + mask->basic.n_proto = cpu_to_be16(~0);
> + } else if (key_val->ppp_proto == htons(PPP_MPLS_MC)) {
> + key->basic.n_proto = htons(ETH_P_MPLS_MC);
> + mask->basic.n_proto = cpu_to_be16(~0);
> + }
> + } else {
> + key->basic.n_proto = 0;
> + mask->basic.n_proto = cpu_to_be16(0);
> + }
> +}
> +
> static void fl_set_key_flag(u32 flower_key, u32 flower_mask,
> u32 *dissector_key, u32 *dissector_mask,
> u32 flower_flag_bit, u32 dissector_flag_bit)
> @@ -1651,6 +1699,9 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
> }
> }
>
> + if (key->basic.n_proto == htons(ETH_P_PPP_SES))
> + fl_set_key_pppoe(tb, &key->pppoe, &mask->pppoe, key, mask);
> +
> if (key->basic.n_proto == htons(ETH_P_IP) ||
> key->basic.n_proto == htons(ETH_P_IPV6)) {
> fl_set_key_val(tb, &key->basic.ip_proto, TCA_FLOWER_KEY_IP_PROTO,
> @@ -1923,6 +1974,8 @@ static void fl_init_dissector(struct flow_dissector *dissector,
> FLOW_DISSECTOR_KEY_HASH, hash);
> FL_KEY_SET_IF_MASKED(mask, keys, cnt,
> FLOW_DISSECTOR_KEY_NUM_OF_VLANS, num_of_vlans);
> + FL_KEY_SET_IF_MASKED(mask, keys, cnt,
> + FLOW_DISSECTOR_KEY_PPPOE, pppoe);
>
> skb_flow_dissector_init(dissector, keys, cnt);
> }
> @@ -3051,6 +3104,14 @@ static int fl_dump_key(struct sk_buff *skb, struct net *net,
> fl_dump_key_ip(skb, false, &key->ip, &mask->ip)))
> goto nla_put_failure;
>
> + if (mask->pppoe.session_id &&
> + nla_put_be16(skb, TCA_FLOWER_KEY_PPPOE_SID, key->pppoe.session_id))
> + goto nla_put_failure;
> +
> + if (mask->basic.n_proto && mask->pppoe.ppp_proto &&
> + nla_put_be16(skb, TCA_FLOWER_KEY_PPP_PROTO, key->pppoe.ppp_proto))
> + goto nla_put_failure;
> +
> if (key->control.addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS &&
> (fl_dump_key_val(skb, &key->ipv4.src, TCA_FLOWER_KEY_IPV4_SRC,
> &mask->ipv4.src, TCA_FLOWER_KEY_IPV4_SRC_MASK,
> --
> 2.35.1
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Intel-wired-lan] [RFC PATCH net-next v4 2/4] net/sched: flower: Add PPPoE filter
@ 2022-07-08 19:22 ` Guillaume Nault
0 siblings, 0 replies; 28+ messages in thread
From: Guillaume Nault @ 2022-07-08 19:22 UTC (permalink / raw)
To: Marcin Szycik
Cc: simon.horman, kurt, paulb, edumazet, boris.sukholitko, jiri,
paulus, jesse.brandeburg, intel-wired-lan, kuba, zhangkaiheb,
pablo, baowen.zheng, komachi.yoshiki, jhs, mostrows,
xiyou.wangcong, pabeni, netdev, gustavoars, davem
On Fri, Jul 08, 2022 at 02:24:19PM +0200, Marcin Szycik wrote:
> From: Wojciech Drewek <wojciech.drewek@intel.com>
>
> Add support for PPPoE specific fields for tc-flower.
> Those fields can be provided only when protocol was set
> to ETH_P_PPP_SES. Defines, dump, load and set are being done here.
>
> Overwrite basic.n_proto only in case of PPP_IP and PPP_IPV6,
... and PPP_MPLS_UC/PPP_MPLS_MC in this new patch version.
> otherwise leave it as ETH_P_PPP_SES.
>
> Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
> ---
> v4:
> * support of MPLS inner fields
> * session_id stored in __be16
>
> include/uapi/linux/pkt_cls.h | 3 ++
> net/sched/cls_flower.c | 61 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 64 insertions(+)
>
> diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
> index 9a2ee1e39fad..c142c0f8ed8a 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, /* be16 */
> + TCA_FLOWER_KEY_PPP_PROTO, /* be16 */
> +
> __TCA_FLOWER_MAX,
> };
>
> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> index dcca70144dff..2a0ebad0e61c 100644
> --- a/net/sched/cls_flower.c
> +++ b/net/sched/cls_flower.c
> @@ -16,6 +16,7 @@
> #include <linux/in6.h>
> #include <linux/ip.h>
> #include <linux/mpls.h>
> +#include <linux/ppp_defs.h>
>
> #include <net/sch_generic.h>
> #include <net/pkt_cls.h>
> @@ -73,6 +74,7 @@ struct fl_flow_key {
> struct flow_dissector_key_ct ct;
> struct flow_dissector_key_hash hash;
> struct flow_dissector_key_num_of_vlans num_of_vlans;
> + struct flow_dissector_key_pppoe pppoe;
> } __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */
>
> struct fl_flow_mask_range {
> @@ -714,6 +716,8 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
> [TCA_FLOWER_KEY_HASH] = { .type = NLA_U32 },
> [TCA_FLOWER_KEY_HASH_MASK] = { .type = NLA_U32 },
> [TCA_FLOWER_KEY_NUM_OF_VLANS] = { .type = NLA_U8 },
> + [TCA_FLOWER_KEY_PPPOE_SID] = { .type = NLA_U16 },
> + [TCA_FLOWER_KEY_PPP_PROTO] = { .type = NLA_U16 },
>
> };
>
> @@ -1041,6 +1045,50 @@ static void fl_set_key_vlan(struct nlattr **tb,
> }
> }
>
> +static void fl_set_key_pppoe(struct nlattr **tb,
> + struct flow_dissector_key_pppoe *key_val,
> + struct flow_dissector_key_pppoe *key_mask,
> + struct fl_flow_key *key,
> + struct fl_flow_key *mask)
> +{
> + /* key_val::type must be set to ETH_P_PPP_SES
> + * because ETH_P_PPP_SES was stored in basic.n_proto
> + * which might get overwritten by ppp_proto
> + * or might be set to 0, the role of key_val::type
> + * is simmilar to vlan_key::tpid
Didn't you mean "vlan_tpid"?
> + */
> + key_val->type = htons(ETH_P_PPP_SES);
> + key_mask->type = cpu_to_be16(~0);
> +
> + if (tb[TCA_FLOWER_KEY_PPPOE_SID]) {
> + key_val->session_id =
> + nla_get_be16(tb[TCA_FLOWER_KEY_PPPOE_SID]);
> + key_mask->session_id = cpu_to_be16(~0);
> + }
> + if (tb[TCA_FLOWER_KEY_PPP_PROTO]) {
> + key_val->ppp_proto =
> + nla_get_be16(tb[TCA_FLOWER_KEY_PPP_PROTO]);
> + key_mask->ppp_proto = cpu_to_be16(~0);
> +
> + if (key_val->ppp_proto == htons(PPP_IP)) {
> + key->basic.n_proto = htons(ETH_P_IP);
> + mask->basic.n_proto = cpu_to_be16(~0);
> + } else if (key_val->ppp_proto == htons(PPP_IPV6)) {
> + key->basic.n_proto = htons(ETH_P_IPV6);
> + mask->basic.n_proto = cpu_to_be16(~0);
> + } else if (key_val->ppp_proto == htons(PPP_MPLS_UC)) {
> + key->basic.n_proto = htons(ETH_P_MPLS_UC);
> + mask->basic.n_proto = cpu_to_be16(~0);
> + } else if (key_val->ppp_proto == htons(PPP_MPLS_MC)) {
> + key->basic.n_proto = htons(ETH_P_MPLS_MC);
> + mask->basic.n_proto = cpu_to_be16(~0);
> + }
> + } else {
> + key->basic.n_proto = 0;
> + mask->basic.n_proto = cpu_to_be16(0);
> + }
> +}
> +
> static void fl_set_key_flag(u32 flower_key, u32 flower_mask,
> u32 *dissector_key, u32 *dissector_mask,
> u32 flower_flag_bit, u32 dissector_flag_bit)
> @@ -1651,6 +1699,9 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
> }
> }
>
> + if (key->basic.n_proto == htons(ETH_P_PPP_SES))
> + fl_set_key_pppoe(tb, &key->pppoe, &mask->pppoe, key, mask);
> +
> if (key->basic.n_proto == htons(ETH_P_IP) ||
> key->basic.n_proto == htons(ETH_P_IPV6)) {
> fl_set_key_val(tb, &key->basic.ip_proto, TCA_FLOWER_KEY_IP_PROTO,
> @@ -1923,6 +1974,8 @@ static void fl_init_dissector(struct flow_dissector *dissector,
> FLOW_DISSECTOR_KEY_HASH, hash);
> FL_KEY_SET_IF_MASKED(mask, keys, cnt,
> FLOW_DISSECTOR_KEY_NUM_OF_VLANS, num_of_vlans);
> + FL_KEY_SET_IF_MASKED(mask, keys, cnt,
> + FLOW_DISSECTOR_KEY_PPPOE, pppoe);
>
> skb_flow_dissector_init(dissector, keys, cnt);
> }
> @@ -3051,6 +3104,14 @@ static int fl_dump_key(struct sk_buff *skb, struct net *net,
> fl_dump_key_ip(skb, false, &key->ip, &mask->ip)))
> goto nla_put_failure;
>
> + if (mask->pppoe.session_id &&
> + nla_put_be16(skb, TCA_FLOWER_KEY_PPPOE_SID, key->pppoe.session_id))
> + goto nla_put_failure;
> +
> + if (mask->basic.n_proto && mask->pppoe.ppp_proto &&
> + nla_put_be16(skb, TCA_FLOWER_KEY_PPP_PROTO, key->pppoe.ppp_proto))
> + goto nla_put_failure;
> +
> if (key->control.addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS &&
> (fl_dump_key_val(skb, &key->ipv4.src, TCA_FLOWER_KEY_IPV4_SRC,
> &mask->ipv4.src, TCA_FLOWER_KEY_IPV4_SRC_MASK,
> --
> 2.35.1
>
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Intel-wired-lan] [RFC PATCH net-next v4 1/4] flow_dissector: Add PPPoE dissectors
2022-07-08 19:05 ` [Intel-wired-lan] " Guillaume Nault
@ 2022-07-11 10:23 ` Drewek, Wojciech
-1 siblings, 0 replies; 28+ messages in thread
From: Drewek, Wojciech @ 2022-07-11 10:23 UTC (permalink / raw)
To: Guillaume Nault, Marcin Szycik
Cc: simon.horman, kurt, paulb, edumazet, boris.sukholitko, paulus,
Brandeburg, Jesse, intel-wired-lan, kuba, zhangkaiheb, pablo,
baowen.zheng, jiri, komachi.yoshiki, jhs, mostrows,
xiyou.wangcong, pabeni, netdev, gustavoars, davem
> -----Original Message-----
> From: Guillaume Nault <gnault@redhat.com>
> Sent: piątek, 8 lipca 2022 21:05
> 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; kuba@kernel.org; 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>; mostrows@earthlink.net;
> paulus@samba.org
> Subject: Re: [RFC PATCH net-next v4 1/4] flow_dissector: Add PPPoE dissectors
>
> On Fri, Jul 08, 2022 at 02:24:18PM +0200, Marcin Szycik wrote:
> > From: Wojciech Drewek <wojciech.drewek@intel.com>
> >
> > Allow to dissect PPPoE specific fields which are:
> > - session ID (16 bits)
> > - ppp protocol (16 bits)
> > - type (16 bits) - this is PPPoE ethertype, for now only
> > ETH_P_PPP_SES is supported, possible ETH_P_PPP_DISC
> > in the future
> >
> > 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>
> > ---
> > v4:
> > * pppoe header validation
> > * added MPLS dissection
> > * added support for compressed ppp protocol field
> > * flow_dissector_key_pppoe::session_id stored in __be16
> > * new field: flow_dissector_key_pppoe::type
> > v3: revert byte order changes in is_ppp_proto_supported from
> > previous version
> > v2: ntohs instead of htons in is_ppp_proto_supported
> >
> > include/net/flow_dissector.h | 13 ++++++
> > net/core/flow_dissector.c | 84 +++++++++++++++++++++++++++++++++---
> > 2 files changed, 90 insertions(+), 7 deletions(-)
> >
> > diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
> > index a4c6057c7097..af0d429b9a26 100644
> > --- a/include/net/flow_dissector.h
> > +++ b/include/net/flow_dissector.h
> > @@ -261,6 +261,18 @@ 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
> > + * @type: pppoe eth type
> > + */
> > +struct flow_dissector_key_pppoe {
> > + __be16 session_id;
> > + __be16 ppp_proto;
> > + __be16 type;
>
> I don't understand the need for the new 'type' field.
Let's say user want to add below filter with just protocol field:
tc filter add dev ens6f0 ingress prio 1 protocol ppp_ses action drop
cls_flower would set basic.n_proto to ETH_P_PPP_SES, then PPPoE packet
arrives with ppp_proto = PPP_IP, which means that in __skb_flow_dissect basic.n_proto is going to
be set to ETH_P_IP. We have a mismatch here cls_flower set basic.n_proto to ETH_P_PPP_SES and
flow_dissector set it to ETH_P_IP. That's why in such example basic.n_proto has to be set to 0 (it works the same
with vlans) and key_pppoe::type has to be used. In other words basic.n_proto can't be used for storing
ETH_P_PPP_SES because it will store encapsulated protocol.
We could also use it to match on ETH_P_PPP_DISC.
>
> > +};
> > +
> > 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 +303,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..3a90219d2354 100644
> > --- a/net/core/flow_dissector.c
> > +++ b/net/core/flow_dissector.c
> > @@ -895,6 +895,42 @@ bool bpf_flow_dissect(struct bpf_prog *prog, struct bpf_flow_dissector *ctx,
> > return result == BPF_OK;
> > }
> >
> > +/**
> > + * is_ppp_proto_supported - checks if inner PPP protocol should be dissected
> > + * @proto: protocol type (PPP proto field)
> > + */
> > +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_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;
> > + }
> > +}
> > +
> > +static bool is_pppoe_ses_hdr_valid(struct pppoe_hdr hdr)
> > +{
> > + return hdr.ver == 1 && hdr.type == 1 && hdr.code == 0;
> > +}
> > +
> > /**
> > * __skb_flow_dissect - extract the flow_keys struct and return it
> > * @net: associated network namespace, derived from @skb if NULL
> > @@ -1214,26 +1250,60 @@ bool __skb_flow_dissect(const struct net *net,
> > struct pppoe_hdr hdr;
> > __be16 proto;
> > } *hdr, _hdr;
> > + __be16 ppp_proto;
> > +
> > hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr);
> > if (!hdr) {
> > fdret = FLOW_DISSECT_RET_OUT_BAD;
> > break;
> > }
> >
> > - nhoff += PPPOE_SES_HLEN;
> > - switch (hdr->proto) {
> > - case htons(PPP_IP):
> > + if (!is_pppoe_ses_hdr_valid(hdr->hdr)) {
> > + fdret = FLOW_DISSECT_RET_OUT_BAD;
> > + break;
> > + }
> > +
> > + /* least significant bit of the first byte
> > + * indicates if protocol field was compressed
> > + */
> > + if (hdr->proto & 1) {
> > + ppp_proto = hdr->proto << 8;
>
> This is little endian specific code. We can't make such assumptions.
Both ppp_proto and hdr->prot are stored in __be16 so left shift by 8 bits
should always be ok, am I right?
Should I use cpu_to_be16 on both 1 and 8. Is that what you mean?
>
> > + nhoff += PPPOE_SES_HLEN - 1;
> > + } else {
> > + ppp_proto = hdr->proto;
> > + nhoff += PPPOE_SES_HLEN;
> > + }
> > +
> > + if (ppp_proto == htons(PPP_IP)) {
> > proto = htons(ETH_P_IP);
> > fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
> > - break;
> > - case htons(PPP_IPV6):
> > + } else if (ppp_proto == htons(PPP_IPV6)) {
> > proto = htons(ETH_P_IPV6);
> > fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
> > - break;
> > - default:
> > + } else if (ppp_proto == htons(PPP_MPLS_UC)) {
> > + proto = htons(ETH_P_MPLS_UC);
> > + fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
> > + } else if (ppp_proto == htons(PPP_MPLS_MC)) {
> > + proto = htons(ETH_P_MPLS_MC);
> > + fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
> > + } else if (is_ppp_proto_supported(ppp_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 = hdr->hdr.sid;
> > + key_pppoe->ppp_proto = ppp_proto;
> > + key_pppoe->type = htons(ETH_P_PPP_SES);
> > + }
> > break;
> > }
> > case htons(ETH_P_TIPC): {
> > --
> > 2.35.1
> >
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 28+ messages in thread
* RE: [RFC PATCH net-next v4 1/4] flow_dissector: Add PPPoE dissectors
@ 2022-07-11 10:23 ` Drewek, Wojciech
0 siblings, 0 replies; 28+ messages in thread
From: Drewek, Wojciech @ 2022-07-11 10:23 UTC (permalink / raw)
To: Guillaume Nault, Marcin Szycik
Cc: netdev, Nguyen, Anthony L, davem, xiyou.wangcong, Brandeburg,
Jesse, gustavoars, baowen.zheng, boris.sukholitko, edumazet,
kuba, jhs, jiri, kurt, pablo, pabeni, paulb, simon.horman,
komachi.yoshiki, zhangkaiheb, intel-wired-lan,
michal.swiatkowski, Lobakin, Alexandr, mostrows, paulus
> -----Original Message-----
> From: Guillaume Nault <gnault@redhat.com>
> Sent: piątek, 8 lipca 2022 21:05
> 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; kuba@kernel.org; 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>; mostrows@earthlink.net;
> paulus@samba.org
> Subject: Re: [RFC PATCH net-next v4 1/4] flow_dissector: Add PPPoE dissectors
>
> On Fri, Jul 08, 2022 at 02:24:18PM +0200, Marcin Szycik wrote:
> > From: Wojciech Drewek <wojciech.drewek@intel.com>
> >
> > Allow to dissect PPPoE specific fields which are:
> > - session ID (16 bits)
> > - ppp protocol (16 bits)
> > - type (16 bits) - this is PPPoE ethertype, for now only
> > ETH_P_PPP_SES is supported, possible ETH_P_PPP_DISC
> > in the future
> >
> > 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>
> > ---
> > v4:
> > * pppoe header validation
> > * added MPLS dissection
> > * added support for compressed ppp protocol field
> > * flow_dissector_key_pppoe::session_id stored in __be16
> > * new field: flow_dissector_key_pppoe::type
> > v3: revert byte order changes in is_ppp_proto_supported from
> > previous version
> > v2: ntohs instead of htons in is_ppp_proto_supported
> >
> > include/net/flow_dissector.h | 13 ++++++
> > net/core/flow_dissector.c | 84 +++++++++++++++++++++++++++++++++---
> > 2 files changed, 90 insertions(+), 7 deletions(-)
> >
> > diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
> > index a4c6057c7097..af0d429b9a26 100644
> > --- a/include/net/flow_dissector.h
> > +++ b/include/net/flow_dissector.h
> > @@ -261,6 +261,18 @@ 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
> > + * @type: pppoe eth type
> > + */
> > +struct flow_dissector_key_pppoe {
> > + __be16 session_id;
> > + __be16 ppp_proto;
> > + __be16 type;
>
> I don't understand the need for the new 'type' field.
Let's say user want to add below filter with just protocol field:
tc filter add dev ens6f0 ingress prio 1 protocol ppp_ses action drop
cls_flower would set basic.n_proto to ETH_P_PPP_SES, then PPPoE packet
arrives with ppp_proto = PPP_IP, which means that in __skb_flow_dissect basic.n_proto is going to
be set to ETH_P_IP. We have a mismatch here cls_flower set basic.n_proto to ETH_P_PPP_SES and
flow_dissector set it to ETH_P_IP. That's why in such example basic.n_proto has to be set to 0 (it works the same
with vlans) and key_pppoe::type has to be used. In other words basic.n_proto can't be used for storing
ETH_P_PPP_SES because it will store encapsulated protocol.
We could also use it to match on ETH_P_PPP_DISC.
>
> > +};
> > +
> > 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 +303,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..3a90219d2354 100644
> > --- a/net/core/flow_dissector.c
> > +++ b/net/core/flow_dissector.c
> > @@ -895,6 +895,42 @@ bool bpf_flow_dissect(struct bpf_prog *prog, struct bpf_flow_dissector *ctx,
> > return result == BPF_OK;
> > }
> >
> > +/**
> > + * is_ppp_proto_supported - checks if inner PPP protocol should be dissected
> > + * @proto: protocol type (PPP proto field)
> > + */
> > +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_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;
> > + }
> > +}
> > +
> > +static bool is_pppoe_ses_hdr_valid(struct pppoe_hdr hdr)
> > +{
> > + return hdr.ver == 1 && hdr.type == 1 && hdr.code == 0;
> > +}
> > +
> > /**
> > * __skb_flow_dissect - extract the flow_keys struct and return it
> > * @net: associated network namespace, derived from @skb if NULL
> > @@ -1214,26 +1250,60 @@ bool __skb_flow_dissect(const struct net *net,
> > struct pppoe_hdr hdr;
> > __be16 proto;
> > } *hdr, _hdr;
> > + __be16 ppp_proto;
> > +
> > hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr);
> > if (!hdr) {
> > fdret = FLOW_DISSECT_RET_OUT_BAD;
> > break;
> > }
> >
> > - nhoff += PPPOE_SES_HLEN;
> > - switch (hdr->proto) {
> > - case htons(PPP_IP):
> > + if (!is_pppoe_ses_hdr_valid(hdr->hdr)) {
> > + fdret = FLOW_DISSECT_RET_OUT_BAD;
> > + break;
> > + }
> > +
> > + /* least significant bit of the first byte
> > + * indicates if protocol field was compressed
> > + */
> > + if (hdr->proto & 1) {
> > + ppp_proto = hdr->proto << 8;
>
> This is little endian specific code. We can't make such assumptions.
Both ppp_proto and hdr->prot are stored in __be16 so left shift by 8 bits
should always be ok, am I right?
Should I use cpu_to_be16 on both 1 and 8. Is that what you mean?
>
> > + nhoff += PPPOE_SES_HLEN - 1;
> > + } else {
> > + ppp_proto = hdr->proto;
> > + nhoff += PPPOE_SES_HLEN;
> > + }
> > +
> > + if (ppp_proto == htons(PPP_IP)) {
> > proto = htons(ETH_P_IP);
> > fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
> > - break;
> > - case htons(PPP_IPV6):
> > + } else if (ppp_proto == htons(PPP_IPV6)) {
> > proto = htons(ETH_P_IPV6);
> > fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
> > - break;
> > - default:
> > + } else if (ppp_proto == htons(PPP_MPLS_UC)) {
> > + proto = htons(ETH_P_MPLS_UC);
> > + fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
> > + } else if (ppp_proto == htons(PPP_MPLS_MC)) {
> > + proto = htons(ETH_P_MPLS_MC);
> > + fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
> > + } else if (is_ppp_proto_supported(ppp_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 = hdr->hdr.sid;
> > + key_pppoe->ppp_proto = ppp_proto;
> > + key_pppoe->type = htons(ETH_P_PPP_SES);
> > + }
> > break;
> > }
> > case htons(ETH_P_TIPC): {
> > --
> > 2.35.1
> >
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Intel-wired-lan] [RFC PATCH net-next v4 2/4] net/sched: flower: Add PPPoE filter
2022-07-08 19:22 ` [Intel-wired-lan] " Guillaume Nault
@ 2022-07-11 10:26 ` Drewek, Wojciech
-1 siblings, 0 replies; 28+ messages in thread
From: Drewek, Wojciech @ 2022-07-11 10:26 UTC (permalink / raw)
To: Guillaume Nault, Marcin Szycik
Cc: simon.horman, kurt, paulb, edumazet, boris.sukholitko, paulus,
Brandeburg, Jesse, intel-wired-lan, kuba, zhangkaiheb, pablo,
baowen.zheng, jiri, komachi.yoshiki, jhs, mostrows,
xiyou.wangcong, pabeni, netdev, gustavoars, davem
> -----Original Message-----
> From: Guillaume Nault <gnault@redhat.com>
> Sent: piątek, 8 lipca 2022 21:23
> 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; kuba@kernel.org; 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>; mostrows@earthlink.net;
> paulus@samba.org
> Subject: Re: [RFC PATCH net-next v4 2/4] net/sched: flower: Add PPPoE filter
>
> On Fri, Jul 08, 2022 at 02:24:19PM +0200, Marcin Szycik wrote:
> > From: Wojciech Drewek <wojciech.drewek@intel.com>
> >
> > Add support for PPPoE specific fields for tc-flower.
> > Those fields can be provided only when protocol was set
> > to ETH_P_PPP_SES. Defines, dump, load and set are being done here.
> >
> > Overwrite basic.n_proto only in case of PPP_IP and PPP_IPV6,
>
> ... and PPP_MPLS_UC/PPP_MPLS_MC in this new patch version.
>
> > otherwise leave it as ETH_P_PPP_SES.
> >
> > Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
> > ---
> > v4:
> > * support of MPLS inner fields
> > * session_id stored in __be16
> >
> > include/uapi/linux/pkt_cls.h | 3 ++
> > net/sched/cls_flower.c | 61 ++++++++++++++++++++++++++++++++++++
> > 2 files changed, 64 insertions(+)
> >
> > diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
> > index 9a2ee1e39fad..c142c0f8ed8a 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, /* be16 */
> > + TCA_FLOWER_KEY_PPP_PROTO, /* be16 */
> > +
> > __TCA_FLOWER_MAX,
> > };
> >
> > diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> > index dcca70144dff..2a0ebad0e61c 100644
> > --- a/net/sched/cls_flower.c
> > +++ b/net/sched/cls_flower.c
> > @@ -16,6 +16,7 @@
> > #include <linux/in6.h>
> > #include <linux/ip.h>
> > #include <linux/mpls.h>
> > +#include <linux/ppp_defs.h>
> >
> > #include <net/sch_generic.h>
> > #include <net/pkt_cls.h>
> > @@ -73,6 +74,7 @@ struct fl_flow_key {
> > struct flow_dissector_key_ct ct;
> > struct flow_dissector_key_hash hash;
> > struct flow_dissector_key_num_of_vlans num_of_vlans;
> > + struct flow_dissector_key_pppoe pppoe;
> > } __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */
> >
> > struct fl_flow_mask_range {
> > @@ -714,6 +716,8 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
> > [TCA_FLOWER_KEY_HASH] = { .type = NLA_U32 },
> > [TCA_FLOWER_KEY_HASH_MASK] = { .type = NLA_U32 },
> > [TCA_FLOWER_KEY_NUM_OF_VLANS] = { .type = NLA_U8 },
> > + [TCA_FLOWER_KEY_PPPOE_SID] = { .type = NLA_U16 },
> > + [TCA_FLOWER_KEY_PPP_PROTO] = { .type = NLA_U16 },
> >
> > };
> >
> > @@ -1041,6 +1045,50 @@ static void fl_set_key_vlan(struct nlattr **tb,
> > }
> > }
> >
> > +static void fl_set_key_pppoe(struct nlattr **tb,
> > + struct flow_dissector_key_pppoe *key_val,
> > + struct flow_dissector_key_pppoe *key_mask,
> > + struct fl_flow_key *key,
> > + struct fl_flow_key *mask)
> > +{
> > + /* key_val::type must be set to ETH_P_PPP_SES
> > + * because ETH_P_PPP_SES was stored in basic.n_proto
> > + * which might get overwritten by ppp_proto
> > + * or might be set to 0, the role of key_val::type
> > + * is simmilar to vlan_key::tpid
>
> Didn't you mean "vlan_tpid"?
Yes, is vlan_key::tpid not clear/valid?
>
> > + */
> > + key_val->type = htons(ETH_P_PPP_SES);
> > + key_mask->type = cpu_to_be16(~0);
> > +
> > + if (tb[TCA_FLOWER_KEY_PPPOE_SID]) {
> > + key_val->session_id =
> > + nla_get_be16(tb[TCA_FLOWER_KEY_PPPOE_SID]);
> > + key_mask->session_id = cpu_to_be16(~0);
> > + }
> > + if (tb[TCA_FLOWER_KEY_PPP_PROTO]) {
> > + key_val->ppp_proto =
> > + nla_get_be16(tb[TCA_FLOWER_KEY_PPP_PROTO]);
> > + key_mask->ppp_proto = cpu_to_be16(~0);
> > +
> > + if (key_val->ppp_proto == htons(PPP_IP)) {
> > + key->basic.n_proto = htons(ETH_P_IP);
> > + mask->basic.n_proto = cpu_to_be16(~0);
> > + } else if (key_val->ppp_proto == htons(PPP_IPV6)) {
> > + key->basic.n_proto = htons(ETH_P_IPV6);
> > + mask->basic.n_proto = cpu_to_be16(~0);
> > + } else if (key_val->ppp_proto == htons(PPP_MPLS_UC)) {
> > + key->basic.n_proto = htons(ETH_P_MPLS_UC);
> > + mask->basic.n_proto = cpu_to_be16(~0);
> > + } else if (key_val->ppp_proto == htons(PPP_MPLS_MC)) {
> > + key->basic.n_proto = htons(ETH_P_MPLS_MC);
> > + mask->basic.n_proto = cpu_to_be16(~0);
> > + }
> > + } else {
> > + key->basic.n_proto = 0;
> > + mask->basic.n_proto = cpu_to_be16(0);
> > + }
> > +}
> > +
> > static void fl_set_key_flag(u32 flower_key, u32 flower_mask,
> > u32 *dissector_key, u32 *dissector_mask,
> > u32 flower_flag_bit, u32 dissector_flag_bit)
> > @@ -1651,6 +1699,9 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
> > }
> > }
> >
> > + if (key->basic.n_proto == htons(ETH_P_PPP_SES))
> > + fl_set_key_pppoe(tb, &key->pppoe, &mask->pppoe, key, mask);
> > +
> > if (key->basic.n_proto == htons(ETH_P_IP) ||
> > key->basic.n_proto == htons(ETH_P_IPV6)) {
> > fl_set_key_val(tb, &key->basic.ip_proto, TCA_FLOWER_KEY_IP_PROTO,
> > @@ -1923,6 +1974,8 @@ static void fl_init_dissector(struct flow_dissector *dissector,
> > FLOW_DISSECTOR_KEY_HASH, hash);
> > FL_KEY_SET_IF_MASKED(mask, keys, cnt,
> > FLOW_DISSECTOR_KEY_NUM_OF_VLANS, num_of_vlans);
> > + FL_KEY_SET_IF_MASKED(mask, keys, cnt,
> > + FLOW_DISSECTOR_KEY_PPPOE, pppoe);
> >
> > skb_flow_dissector_init(dissector, keys, cnt);
> > }
> > @@ -3051,6 +3104,14 @@ static int fl_dump_key(struct sk_buff *skb, struct net *net,
> > fl_dump_key_ip(skb, false, &key->ip, &mask->ip)))
> > goto nla_put_failure;
> >
> > + if (mask->pppoe.session_id &&
> > + nla_put_be16(skb, TCA_FLOWER_KEY_PPPOE_SID, key->pppoe.session_id))
> > + goto nla_put_failure;
> > +
> > + if (mask->basic.n_proto && mask->pppoe.ppp_proto &&
> > + nla_put_be16(skb, TCA_FLOWER_KEY_PPP_PROTO, key->pppoe.ppp_proto))
> > + goto nla_put_failure;
> > +
> > if (key->control.addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS &&
> > (fl_dump_key_val(skb, &key->ipv4.src, TCA_FLOWER_KEY_IPV4_SRC,
> > &mask->ipv4.src, TCA_FLOWER_KEY_IPV4_SRC_MASK,
> > --
> > 2.35.1
> >
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 28+ messages in thread
* RE: [RFC PATCH net-next v4 2/4] net/sched: flower: Add PPPoE filter
@ 2022-07-11 10:26 ` Drewek, Wojciech
0 siblings, 0 replies; 28+ messages in thread
From: Drewek, Wojciech @ 2022-07-11 10:26 UTC (permalink / raw)
To: Guillaume Nault, Marcin Szycik
Cc: netdev, Nguyen, Anthony L, davem, xiyou.wangcong, Brandeburg,
Jesse, gustavoars, baowen.zheng, boris.sukholitko, edumazet,
kuba, jhs, jiri, kurt, pablo, pabeni, paulb, simon.horman,
komachi.yoshiki, zhangkaiheb, intel-wired-lan,
michal.swiatkowski, Lobakin, Alexandr, mostrows, paulus
> -----Original Message-----
> From: Guillaume Nault <gnault@redhat.com>
> Sent: piątek, 8 lipca 2022 21:23
> 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; kuba@kernel.org; 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>; mostrows@earthlink.net;
> paulus@samba.org
> Subject: Re: [RFC PATCH net-next v4 2/4] net/sched: flower: Add PPPoE filter
>
> On Fri, Jul 08, 2022 at 02:24:19PM +0200, Marcin Szycik wrote:
> > From: Wojciech Drewek <wojciech.drewek@intel.com>
> >
> > Add support for PPPoE specific fields for tc-flower.
> > Those fields can be provided only when protocol was set
> > to ETH_P_PPP_SES. Defines, dump, load and set are being done here.
> >
> > Overwrite basic.n_proto only in case of PPP_IP and PPP_IPV6,
>
> ... and PPP_MPLS_UC/PPP_MPLS_MC in this new patch version.
>
> > otherwise leave it as ETH_P_PPP_SES.
> >
> > Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
> > ---
> > v4:
> > * support of MPLS inner fields
> > * session_id stored in __be16
> >
> > include/uapi/linux/pkt_cls.h | 3 ++
> > net/sched/cls_flower.c | 61 ++++++++++++++++++++++++++++++++++++
> > 2 files changed, 64 insertions(+)
> >
> > diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
> > index 9a2ee1e39fad..c142c0f8ed8a 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, /* be16 */
> > + TCA_FLOWER_KEY_PPP_PROTO, /* be16 */
> > +
> > __TCA_FLOWER_MAX,
> > };
> >
> > diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> > index dcca70144dff..2a0ebad0e61c 100644
> > --- a/net/sched/cls_flower.c
> > +++ b/net/sched/cls_flower.c
> > @@ -16,6 +16,7 @@
> > #include <linux/in6.h>
> > #include <linux/ip.h>
> > #include <linux/mpls.h>
> > +#include <linux/ppp_defs.h>
> >
> > #include <net/sch_generic.h>
> > #include <net/pkt_cls.h>
> > @@ -73,6 +74,7 @@ struct fl_flow_key {
> > struct flow_dissector_key_ct ct;
> > struct flow_dissector_key_hash hash;
> > struct flow_dissector_key_num_of_vlans num_of_vlans;
> > + struct flow_dissector_key_pppoe pppoe;
> > } __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */
> >
> > struct fl_flow_mask_range {
> > @@ -714,6 +716,8 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
> > [TCA_FLOWER_KEY_HASH] = { .type = NLA_U32 },
> > [TCA_FLOWER_KEY_HASH_MASK] = { .type = NLA_U32 },
> > [TCA_FLOWER_KEY_NUM_OF_VLANS] = { .type = NLA_U8 },
> > + [TCA_FLOWER_KEY_PPPOE_SID] = { .type = NLA_U16 },
> > + [TCA_FLOWER_KEY_PPP_PROTO] = { .type = NLA_U16 },
> >
> > };
> >
> > @@ -1041,6 +1045,50 @@ static void fl_set_key_vlan(struct nlattr **tb,
> > }
> > }
> >
> > +static void fl_set_key_pppoe(struct nlattr **tb,
> > + struct flow_dissector_key_pppoe *key_val,
> > + struct flow_dissector_key_pppoe *key_mask,
> > + struct fl_flow_key *key,
> > + struct fl_flow_key *mask)
> > +{
> > + /* key_val::type must be set to ETH_P_PPP_SES
> > + * because ETH_P_PPP_SES was stored in basic.n_proto
> > + * which might get overwritten by ppp_proto
> > + * or might be set to 0, the role of key_val::type
> > + * is simmilar to vlan_key::tpid
>
> Didn't you mean "vlan_tpid"?
Yes, is vlan_key::tpid not clear/valid?
>
> > + */
> > + key_val->type = htons(ETH_P_PPP_SES);
> > + key_mask->type = cpu_to_be16(~0);
> > +
> > + if (tb[TCA_FLOWER_KEY_PPPOE_SID]) {
> > + key_val->session_id =
> > + nla_get_be16(tb[TCA_FLOWER_KEY_PPPOE_SID]);
> > + key_mask->session_id = cpu_to_be16(~0);
> > + }
> > + if (tb[TCA_FLOWER_KEY_PPP_PROTO]) {
> > + key_val->ppp_proto =
> > + nla_get_be16(tb[TCA_FLOWER_KEY_PPP_PROTO]);
> > + key_mask->ppp_proto = cpu_to_be16(~0);
> > +
> > + if (key_val->ppp_proto == htons(PPP_IP)) {
> > + key->basic.n_proto = htons(ETH_P_IP);
> > + mask->basic.n_proto = cpu_to_be16(~0);
> > + } else if (key_val->ppp_proto == htons(PPP_IPV6)) {
> > + key->basic.n_proto = htons(ETH_P_IPV6);
> > + mask->basic.n_proto = cpu_to_be16(~0);
> > + } else if (key_val->ppp_proto == htons(PPP_MPLS_UC)) {
> > + key->basic.n_proto = htons(ETH_P_MPLS_UC);
> > + mask->basic.n_proto = cpu_to_be16(~0);
> > + } else if (key_val->ppp_proto == htons(PPP_MPLS_MC)) {
> > + key->basic.n_proto = htons(ETH_P_MPLS_MC);
> > + mask->basic.n_proto = cpu_to_be16(~0);
> > + }
> > + } else {
> > + key->basic.n_proto = 0;
> > + mask->basic.n_proto = cpu_to_be16(0);
> > + }
> > +}
> > +
> > static void fl_set_key_flag(u32 flower_key, u32 flower_mask,
> > u32 *dissector_key, u32 *dissector_mask,
> > u32 flower_flag_bit, u32 dissector_flag_bit)
> > @@ -1651,6 +1699,9 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
> > }
> > }
> >
> > + if (key->basic.n_proto == htons(ETH_P_PPP_SES))
> > + fl_set_key_pppoe(tb, &key->pppoe, &mask->pppoe, key, mask);
> > +
> > if (key->basic.n_proto == htons(ETH_P_IP) ||
> > key->basic.n_proto == htons(ETH_P_IPV6)) {
> > fl_set_key_val(tb, &key->basic.ip_proto, TCA_FLOWER_KEY_IP_PROTO,
> > @@ -1923,6 +1974,8 @@ static void fl_init_dissector(struct flow_dissector *dissector,
> > FLOW_DISSECTOR_KEY_HASH, hash);
> > FL_KEY_SET_IF_MASKED(mask, keys, cnt,
> > FLOW_DISSECTOR_KEY_NUM_OF_VLANS, num_of_vlans);
> > + FL_KEY_SET_IF_MASKED(mask, keys, cnt,
> > + FLOW_DISSECTOR_KEY_PPPOE, pppoe);
> >
> > skb_flow_dissector_init(dissector, keys, cnt);
> > }
> > @@ -3051,6 +3104,14 @@ static int fl_dump_key(struct sk_buff *skb, struct net *net,
> > fl_dump_key_ip(skb, false, &key->ip, &mask->ip)))
> > goto nla_put_failure;
> >
> > + if (mask->pppoe.session_id &&
> > + nla_put_be16(skb, TCA_FLOWER_KEY_PPPOE_SID, key->pppoe.session_id))
> > + goto nla_put_failure;
> > +
> > + if (mask->basic.n_proto && mask->pppoe.ppp_proto &&
> > + nla_put_be16(skb, TCA_FLOWER_KEY_PPP_PROTO, key->pppoe.ppp_proto))
> > + goto nla_put_failure;
> > +
> > if (key->control.addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS &&
> > (fl_dump_key_val(skb, &key->ipv4.src, TCA_FLOWER_KEY_IPV4_SRC,
> > &mask->ipv4.src, TCA_FLOWER_KEY_IPV4_SRC_MASK,
> > --
> > 2.35.1
> >
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH net-next v4 1/4] flow_dissector: Add PPPoE dissectors
2022-07-11 10:23 ` Drewek, Wojciech
@ 2022-07-12 17:20 ` Guillaume Nault
-1 siblings, 0 replies; 28+ messages in thread
From: Guillaume Nault @ 2022-07-12 17:20 UTC (permalink / raw)
To: Drewek, Wojciech
Cc: Marcin Szycik, netdev, Nguyen, Anthony L, davem, xiyou.wangcong,
Brandeburg, Jesse, gustavoars, baowen.zheng, boris.sukholitko,
edumazet, kuba, jhs, jiri, kurt, pablo, pabeni, paulb,
simon.horman, komachi.yoshiki, zhangkaiheb, intel-wired-lan,
michal.swiatkowski, Lobakin, Alexandr, mostrows, paulus
On Mon, Jul 11, 2022 at 10:23:50AM +0000, Drewek, Wojciech wrote:
> > > diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
> > > index a4c6057c7097..af0d429b9a26 100644
> > > --- a/include/net/flow_dissector.h
> > > +++ b/include/net/flow_dissector.h
> > > @@ -261,6 +261,18 @@ 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
> > > + * @type: pppoe eth type
> > > + */
> > > +struct flow_dissector_key_pppoe {
> > > + __be16 session_id;
> > > + __be16 ppp_proto;
> > > + __be16 type;
> >
> > I don't understand the need for the new 'type' field.
>
> Let's say user want to add below filter with just protocol field:
> tc filter add dev ens6f0 ingress prio 1 protocol ppp_ses action drop
>
> cls_flower would set basic.n_proto to ETH_P_PPP_SES, then PPPoE packet
> arrives with ppp_proto = PPP_IP, which means that in __skb_flow_dissect basic.n_proto is going to
> be set to ETH_P_IP. We have a mismatch here cls_flower set basic.n_proto to ETH_P_PPP_SES and
> flow_dissector set it to ETH_P_IP. That's why in such example basic.n_proto has to be set to 0 (it works the same
> with vlans) and key_pppoe::type has to be used. In other words basic.n_proto can't be used for storing
> ETH_P_PPP_SES because it will store encapsulated protocol.
>
> We could also use it to match on ETH_P_PPP_DISC.
Thanks for the explanation. That makes sense.
> > > @@ -1214,26 +1250,60 @@ bool __skb_flow_dissect(const struct net *net,
> > > struct pppoe_hdr hdr;
> > > __be16 proto;
> > > } *hdr, _hdr;
> > > + __be16 ppp_proto;
> > > +
> > > hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr);
> > > if (!hdr) {
> > > fdret = FLOW_DISSECT_RET_OUT_BAD;
> > > break;
> > > }
> > >
> > > - nhoff += PPPOE_SES_HLEN;
> > > - switch (hdr->proto) {
> > > - case htons(PPP_IP):
> > > + if (!is_pppoe_ses_hdr_valid(hdr->hdr)) {
> > > + fdret = FLOW_DISSECT_RET_OUT_BAD;
> > > + break;
> > > + }
> > > +
> > > + /* least significant bit of the first byte
> > > + * indicates if protocol field was compressed
> > > + */
> > > + if (hdr->proto & 1) {
> > > + ppp_proto = hdr->proto << 8;
> >
> > This is little endian specific code. We can't make such assumptions.
>
> Both ppp_proto and hdr->prot are stored in __be16 so left shift by 8 bits
> should always be ok, am I right?
Sorry, I don't understand. How could the test and the bit shift
operation give the correct result on a big endian machine?
Let's say we handle an IPv4 packet and the PPP protocol field isn't
compressed. That is, protocol is 0x0021.
On a big endian machine 'hdr->proto & 1' is true and the bit shift sets
ppp_proto to 0x2100, while the code should have left the original value
untouched.
> Should I use cpu_to_be16 on both 1 and 8. Is that what you mean?
I can't see how cpu_to_be16() could help here. I was thinking of simply
using ntohs(hdr->proto).
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Intel-wired-lan] [RFC PATCH net-next v4 1/4] flow_dissector: Add PPPoE dissectors
@ 2022-07-12 17:20 ` Guillaume Nault
0 siblings, 0 replies; 28+ messages in thread
From: Guillaume Nault @ 2022-07-12 17:20 UTC (permalink / raw)
To: Drewek, Wojciech
Cc: simon.horman, kurt, paulb, edumazet, boris.sukholitko, paulus,
Brandeburg, Jesse, intel-wired-lan, kuba, zhangkaiheb, pablo,
baowen.zheng, jiri, komachi.yoshiki, jhs, mostrows,
xiyou.wangcong, pabeni, netdev, gustavoars, davem
On Mon, Jul 11, 2022 at 10:23:50AM +0000, Drewek, Wojciech wrote:
> > > diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
> > > index a4c6057c7097..af0d429b9a26 100644
> > > --- a/include/net/flow_dissector.h
> > > +++ b/include/net/flow_dissector.h
> > > @@ -261,6 +261,18 @@ 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
> > > + * @type: pppoe eth type
> > > + */
> > > +struct flow_dissector_key_pppoe {
> > > + __be16 session_id;
> > > + __be16 ppp_proto;
> > > + __be16 type;
> >
> > I don't understand the need for the new 'type' field.
>
> Let's say user want to add below filter with just protocol field:
> tc filter add dev ens6f0 ingress prio 1 protocol ppp_ses action drop
>
> cls_flower would set basic.n_proto to ETH_P_PPP_SES, then PPPoE packet
> arrives with ppp_proto = PPP_IP, which means that in __skb_flow_dissect basic.n_proto is going to
> be set to ETH_P_IP. We have a mismatch here cls_flower set basic.n_proto to ETH_P_PPP_SES and
> flow_dissector set it to ETH_P_IP. That's why in such example basic.n_proto has to be set to 0 (it works the same
> with vlans) and key_pppoe::type has to be used. In other words basic.n_proto can't be used for storing
> ETH_P_PPP_SES because it will store encapsulated protocol.
>
> We could also use it to match on ETH_P_PPP_DISC.
Thanks for the explanation. That makes sense.
> > > @@ -1214,26 +1250,60 @@ bool __skb_flow_dissect(const struct net *net,
> > > struct pppoe_hdr hdr;
> > > __be16 proto;
> > > } *hdr, _hdr;
> > > + __be16 ppp_proto;
> > > +
> > > hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr);
> > > if (!hdr) {
> > > fdret = FLOW_DISSECT_RET_OUT_BAD;
> > > break;
> > > }
> > >
> > > - nhoff += PPPOE_SES_HLEN;
> > > - switch (hdr->proto) {
> > > - case htons(PPP_IP):
> > > + if (!is_pppoe_ses_hdr_valid(hdr->hdr)) {
> > > + fdret = FLOW_DISSECT_RET_OUT_BAD;
> > > + break;
> > > + }
> > > +
> > > + /* least significant bit of the first byte
> > > + * indicates if protocol field was compressed
> > > + */
> > > + if (hdr->proto & 1) {
> > > + ppp_proto = hdr->proto << 8;
> >
> > This is little endian specific code. We can't make such assumptions.
>
> Both ppp_proto and hdr->prot are stored in __be16 so left shift by 8 bits
> should always be ok, am I right?
Sorry, I don't understand. How could the test and the bit shift
operation give the correct result on a big endian machine?
Let's say we handle an IPv4 packet and the PPP protocol field isn't
compressed. That is, protocol is 0x0021.
On a big endian machine 'hdr->proto & 1' is true and the bit shift sets
ppp_proto to 0x2100, while the code should have left the original value
untouched.
> Should I use cpu_to_be16 on both 1 and 8. Is that what you mean?
I can't see how cpu_to_be16() could help here. I was thinking of simply
using ntohs(hdr->proto).
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH net-next v4 2/4] net/sched: flower: Add PPPoE filter
2022-07-11 10:26 ` Drewek, Wojciech
@ 2022-07-12 17:31 ` Guillaume Nault
-1 siblings, 0 replies; 28+ messages in thread
From: Guillaume Nault @ 2022-07-12 17:31 UTC (permalink / raw)
To: Drewek, Wojciech
Cc: Marcin Szycik, netdev, Nguyen, Anthony L, davem, xiyou.wangcong,
Brandeburg, Jesse, gustavoars, baowen.zheng, boris.sukholitko,
edumazet, kuba, jhs, jiri, kurt, pablo, pabeni, paulb,
simon.horman, komachi.yoshiki, zhangkaiheb, intel-wired-lan,
michal.swiatkowski, Lobakin, Alexandr, mostrows, paulus
On Mon, Jul 11, 2022 at 10:26:21AM +0000, Drewek, Wojciech wrote:
> > > +static void fl_set_key_pppoe(struct nlattr **tb,
> > > + struct flow_dissector_key_pppoe *key_val,
> > > + struct flow_dissector_key_pppoe *key_mask,
> > > + struct fl_flow_key *key,
> > > + struct fl_flow_key *mask)
> > > +{
> > > + /* key_val::type must be set to ETH_P_PPP_SES
> > > + * because ETH_P_PPP_SES was stored in basic.n_proto
> > > + * which might get overwritten by ppp_proto
> > > + * or might be set to 0, the role of key_val::type
> > > + * is simmilar to vlan_key::tpid
> >
> > Didn't you mean "vlan_tpid"?
>
> Yes, is vlan_key::tpid not clear/valid?
At least it wasn't entirely clear to me as I wondered if I got the
comment right. And it's basically free to use the real name of the
structure field.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Intel-wired-lan] [RFC PATCH net-next v4 2/4] net/sched: flower: Add PPPoE filter
@ 2022-07-12 17:31 ` Guillaume Nault
0 siblings, 0 replies; 28+ messages in thread
From: Guillaume Nault @ 2022-07-12 17:31 UTC (permalink / raw)
To: Drewek, Wojciech
Cc: simon.horman, kurt, paulb, edumazet, boris.sukholitko, paulus,
Brandeburg, Jesse, intel-wired-lan, kuba, zhangkaiheb, pablo,
baowen.zheng, jiri, komachi.yoshiki, jhs, mostrows,
xiyou.wangcong, pabeni, netdev, gustavoars, davem
On Mon, Jul 11, 2022 at 10:26:21AM +0000, Drewek, Wojciech wrote:
> > > +static void fl_set_key_pppoe(struct nlattr **tb,
> > > + struct flow_dissector_key_pppoe *key_val,
> > > + struct flow_dissector_key_pppoe *key_mask,
> > > + struct fl_flow_key *key,
> > > + struct fl_flow_key *mask)
> > > +{
> > > + /* key_val::type must be set to ETH_P_PPP_SES
> > > + * because ETH_P_PPP_SES was stored in basic.n_proto
> > > + * which might get overwritten by ppp_proto
> > > + * or might be set to 0, the role of key_val::type
> > > + * is simmilar to vlan_key::tpid
> >
> > Didn't you mean "vlan_tpid"?
>
> Yes, is vlan_key::tpid not clear/valid?
At least it wasn't entirely clear to me as I wondered if I got the
comment right. And it's basically free to use the real name of the
structure field.
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 28+ messages in thread
* RE: [RFC PATCH net-next v4 1/4] flow_dissector: Add PPPoE dissectors
2022-07-12 17:20 ` [Intel-wired-lan] " Guillaume Nault
@ 2022-07-13 7:58 ` Drewek, Wojciech
-1 siblings, 0 replies; 28+ messages in thread
From: Drewek, Wojciech @ 2022-07-13 7:58 UTC (permalink / raw)
To: Guillaume Nault
Cc: Marcin Szycik, netdev, Nguyen, Anthony L, davem, xiyou.wangcong,
Brandeburg, Jesse, gustavoars, baowen.zheng, boris.sukholitko,
edumazet, kuba, jhs, jiri, kurt, pablo, pabeni, paulb,
simon.horman, komachi.yoshiki, zhangkaiheb, intel-wired-lan,
michal.swiatkowski, Lobakin, Alexandr, mostrows, paulus
> -----Original Message-----
> From: Guillaume Nault <gnault@redhat.com>
> Sent: wtorek, 12 lipca 2022 19: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; kuba@kernel.org; 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>; mostrows@earthlink.net; paulus@samba.org
> Subject: Re: [RFC PATCH net-next v4 1/4] flow_dissector: Add PPPoE dissectors
>
> On Mon, Jul 11, 2022 at 10:23:50AM +0000, Drewek, Wojciech wrote:
> > > > diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
> > > > index a4c6057c7097..af0d429b9a26 100644
> > > > --- a/include/net/flow_dissector.h
> > > > +++ b/include/net/flow_dissector.h
> > > > @@ -261,6 +261,18 @@ 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
> > > > + * @type: pppoe eth type
> > > > + */
> > > > +struct flow_dissector_key_pppoe {
> > > > + __be16 session_id;
> > > > + __be16 ppp_proto;
> > > > + __be16 type;
> > >
> > > I don't understand the need for the new 'type' field.
> >
> > Let's say user want to add below filter with just protocol field:
> > tc filter add dev ens6f0 ingress prio 1 protocol ppp_ses action drop
> >
> > cls_flower would set basic.n_proto to ETH_P_PPP_SES, then PPPoE packet
> > arrives with ppp_proto = PPP_IP, which means that in __skb_flow_dissect basic.n_proto is going to
> > be set to ETH_P_IP. We have a mismatch here cls_flower set basic.n_proto to ETH_P_PPP_SES and
> > flow_dissector set it to ETH_P_IP. That's why in such example basic.n_proto has to be set to 0 (it works the same
> > with vlans) and key_pppoe::type has to be used. In other words basic.n_proto can't be used for storing
> > ETH_P_PPP_SES because it will store encapsulated protocol.
> >
> > We could also use it to match on ETH_P_PPP_DISC.
>
> Thanks for the explanation. That makes sense.
>
> > > > @@ -1214,26 +1250,60 @@ bool __skb_flow_dissect(const struct net *net,
> > > > struct pppoe_hdr hdr;
> > > > __be16 proto;
> > > > } *hdr, _hdr;
> > > > + __be16 ppp_proto;
> > > > +
> > > > hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr);
> > > > if (!hdr) {
> > > > fdret = FLOW_DISSECT_RET_OUT_BAD;
> > > > break;
> > > > }
> > > >
> > > > - nhoff += PPPOE_SES_HLEN;
> > > > - switch (hdr->proto) {
> > > > - case htons(PPP_IP):
> > > > + if (!is_pppoe_ses_hdr_valid(hdr->hdr)) {
> > > > + fdret = FLOW_DISSECT_RET_OUT_BAD;
> > > > + break;
> > > > + }
> > > > +
> > > > + /* least significant bit of the first byte
> > > > + * indicates if protocol field was compressed
> > > > + */
> > > > + if (hdr->proto & 1) {
> > > > + ppp_proto = hdr->proto << 8;
> > >
> > > This is little endian specific code. We can't make such assumptions.
> >
> > Both ppp_proto and hdr->prot are stored in __be16 so left shift by 8 bits
> > should always be ok, am I right?
>
> Sorry, I don't understand. How could the test and the bit shift
> operation give the correct result on a big endian machine?
>
> Let's say we handle an IPv4 packet and the PPP protocol field isn't
> compressed. That is, protocol is 0x0021.
> On a big endian machine 'hdr->proto & 1' is true and the bit shift sets
> ppp_proto to 0x2100, while the code should have left the original value
> untouched.
>
Ok, I see now, we'll fix it in the next version.
> > Should I use cpu_to_be16 on both 1 and 8. Is that what you mean?
>
> I can't see how cpu_to_be16() could help here. I was thinking of simply
> using ntohs(hdr->proto).
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Intel-wired-lan] [RFC PATCH net-next v4 1/4] flow_dissector: Add PPPoE dissectors
@ 2022-07-13 7:58 ` Drewek, Wojciech
0 siblings, 0 replies; 28+ messages in thread
From: Drewek, Wojciech @ 2022-07-13 7:58 UTC (permalink / raw)
To: Guillaume Nault
Cc: simon.horman, kurt, paulb, edumazet, boris.sukholitko, paulus,
Brandeburg, Jesse, intel-wired-lan, kuba, zhangkaiheb, pablo,
baowen.zheng, jiri, komachi.yoshiki, jhs, mostrows,
xiyou.wangcong, pabeni, netdev, gustavoars, davem
> -----Original Message-----
> From: Guillaume Nault <gnault@redhat.com>
> Sent: wtorek, 12 lipca 2022 19: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; kuba@kernel.org; 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>; mostrows@earthlink.net; paulus@samba.org
> Subject: Re: [RFC PATCH net-next v4 1/4] flow_dissector: Add PPPoE dissectors
>
> On Mon, Jul 11, 2022 at 10:23:50AM +0000, Drewek, Wojciech wrote:
> > > > diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
> > > > index a4c6057c7097..af0d429b9a26 100644
> > > > --- a/include/net/flow_dissector.h
> > > > +++ b/include/net/flow_dissector.h
> > > > @@ -261,6 +261,18 @@ 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
> > > > + * @type: pppoe eth type
> > > > + */
> > > > +struct flow_dissector_key_pppoe {
> > > > + __be16 session_id;
> > > > + __be16 ppp_proto;
> > > > + __be16 type;
> > >
> > > I don't understand the need for the new 'type' field.
> >
> > Let's say user want to add below filter with just protocol field:
> > tc filter add dev ens6f0 ingress prio 1 protocol ppp_ses action drop
> >
> > cls_flower would set basic.n_proto to ETH_P_PPP_SES, then PPPoE packet
> > arrives with ppp_proto = PPP_IP, which means that in __skb_flow_dissect basic.n_proto is going to
> > be set to ETH_P_IP. We have a mismatch here cls_flower set basic.n_proto to ETH_P_PPP_SES and
> > flow_dissector set it to ETH_P_IP. That's why in such example basic.n_proto has to be set to 0 (it works the same
> > with vlans) and key_pppoe::type has to be used. In other words basic.n_proto can't be used for storing
> > ETH_P_PPP_SES because it will store encapsulated protocol.
> >
> > We could also use it to match on ETH_P_PPP_DISC.
>
> Thanks for the explanation. That makes sense.
>
> > > > @@ -1214,26 +1250,60 @@ bool __skb_flow_dissect(const struct net *net,
> > > > struct pppoe_hdr hdr;
> > > > __be16 proto;
> > > > } *hdr, _hdr;
> > > > + __be16 ppp_proto;
> > > > +
> > > > hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr);
> > > > if (!hdr) {
> > > > fdret = FLOW_DISSECT_RET_OUT_BAD;
> > > > break;
> > > > }
> > > >
> > > > - nhoff += PPPOE_SES_HLEN;
> > > > - switch (hdr->proto) {
> > > > - case htons(PPP_IP):
> > > > + if (!is_pppoe_ses_hdr_valid(hdr->hdr)) {
> > > > + fdret = FLOW_DISSECT_RET_OUT_BAD;
> > > > + break;
> > > > + }
> > > > +
> > > > + /* least significant bit of the first byte
> > > > + * indicates if protocol field was compressed
> > > > + */
> > > > + if (hdr->proto & 1) {
> > > > + ppp_proto = hdr->proto << 8;
> > >
> > > This is little endian specific code. We can't make such assumptions.
> >
> > Both ppp_proto and hdr->prot are stored in __be16 so left shift by 8 bits
> > should always be ok, am I right?
>
> Sorry, I don't understand. How could the test and the bit shift
> operation give the correct result on a big endian machine?
>
> Let's say we handle an IPv4 packet and the PPP protocol field isn't
> compressed. That is, protocol is 0x0021.
> On a big endian machine 'hdr->proto & 1' is true and the bit shift sets
> ppp_proto to 0x2100, while the code should have left the original value
> untouched.
>
Ok, I see now, we'll fix it in the next version.
> > Should I use cpu_to_be16 on both 1 and 8. Is that what you mean?
>
> I can't see how cpu_to_be16() could help here. I was thinking of simply
> using ntohs(hdr->proto).
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 28+ messages in thread
* RE: [RFC PATCH net-next v4 1/4] flow_dissector: Add PPPoE dissectors
2022-07-13 7:58 ` [Intel-wired-lan] " Drewek, Wojciech
@ 2022-07-13 13:54 ` Drewek, Wojciech
-1 siblings, 0 replies; 28+ messages in thread
From: Drewek, Wojciech @ 2022-07-13 13:54 UTC (permalink / raw)
To: Guillaume Nault
Cc: Marcin Szycik, netdev, Nguyen, Anthony L, davem, xiyou.wangcong,
Brandeburg, Jesse, gustavoars, baowen.zheng, boris.sukholitko,
edumazet, kuba, jhs, jiri, kurt, pablo, pabeni, paulb,
simon.horman, komachi.yoshiki, zhangkaiheb, intel-wired-lan,
michal.swiatkowski, Lobakin, Alexandr, mostrows, paulus
> -----Original Message-----
> From: Drewek, Wojciech
> Sent: środa, 13 lipca 2022 09:59
> To: Guillaume Nault <gnault@redhat.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; kuba@kernel.org; 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>; mostrows@earthlink.net; paulus@samba.org
> Subject: RE: [RFC PATCH net-next v4 1/4] flow_dissector: Add PPPoE dissectors
>
>
>
> > -----Original Message-----
> > From: Guillaume Nault <gnault@redhat.com>
> > Sent: wtorek, 12 lipca 2022 19: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; kuba@kernel.org; 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>; mostrows@earthlink.net; paulus@samba.org
> > Subject: Re: [RFC PATCH net-next v4 1/4] flow_dissector: Add PPPoE dissectors
> >
> > On Mon, Jul 11, 2022 at 10:23:50AM +0000, Drewek, Wojciech wrote:
> > > > > diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
> > > > > index a4c6057c7097..af0d429b9a26 100644
> > > > > --- a/include/net/flow_dissector.h
> > > > > +++ b/include/net/flow_dissector.h
> > > > > @@ -261,6 +261,18 @@ 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
> > > > > + * @type: pppoe eth type
> > > > > + */
> > > > > +struct flow_dissector_key_pppoe {
> > > > > + __be16 session_id;
> > > > > + __be16 ppp_proto;
> > > > > + __be16 type;
> > > >
> > > > I don't understand the need for the new 'type' field.
> > >
> > > Let's say user want to add below filter with just protocol field:
> > > tc filter add dev ens6f0 ingress prio 1 protocol ppp_ses action drop
> > >
> > > cls_flower would set basic.n_proto to ETH_P_PPP_SES, then PPPoE packet
> > > arrives with ppp_proto = PPP_IP, which means that in __skb_flow_dissect basic.n_proto is going to
> > > be set to ETH_P_IP. We have a mismatch here cls_flower set basic.n_proto to ETH_P_PPP_SES and
> > > flow_dissector set it to ETH_P_IP. That's why in such example basic.n_proto has to be set to 0 (it works the same
> > > with vlans) and key_pppoe::type has to be used. In other words basic.n_proto can't be used for storing
> > > ETH_P_PPP_SES because it will store encapsulated protocol.
> > >
> > > We could also use it to match on ETH_P_PPP_DISC.
> >
> > Thanks for the explanation. That makes sense.
> >
> > > > > @@ -1214,26 +1250,60 @@ bool __skb_flow_dissect(const struct net *net,
> > > > > struct pppoe_hdr hdr;
> > > > > __be16 proto;
> > > > > } *hdr, _hdr;
> > > > > + __be16 ppp_proto;
> > > > > +
> > > > > hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr);
> > > > > if (!hdr) {
> > > > > fdret = FLOW_DISSECT_RET_OUT_BAD;
> > > > > break;
> > > > > }
> > > > >
> > > > > - nhoff += PPPOE_SES_HLEN;
> > > > > - switch (hdr->proto) {
> > > > > - case htons(PPP_IP):
> > > > > + if (!is_pppoe_ses_hdr_valid(hdr->hdr)) {
> > > > > + fdret = FLOW_DISSECT_RET_OUT_BAD;
> > > > > + break;
> > > > > + }
> > > > > +
> > > > > + /* least significant bit of the first byte
> > > > > + * indicates if protocol field was compressed
> > > > > + */
> > > > > + if (hdr->proto & 1) {
> > > > > + ppp_proto = hdr->proto << 8;
> > > >
> > > > This is little endian specific code. We can't make such assumptions.
> > >
> > > Both ppp_proto and hdr->prot are stored in __be16 so left shift by 8 bits
> > > should always be ok, am I right?
> >
> > Sorry, I don't understand. How could the test and the bit shift
> > operation give the correct result on a big endian machine?
> >
> > Let's say we handle an IPv4 packet and the PPP protocol field isn't
> > compressed. That is, protocol is 0x0021.
> > On a big endian machine 'hdr->proto & 1' is true and the bit shift sets
> > ppp_proto to 0x2100, while the code should have left the original value
> > untouched.
> >
>
> Ok, I see now, we'll fix it in the next version.
I think this should work with both LE and BE arch, what do you think Guillaume?
We don't want to spam so much with next versions so maybe it is better
to ask earlier.
u16 ppp_proto;
ppp_proto = ntohs(hdr->proto);
if (ppp_proto & 256) {
ppp_proto = htons(ppp_proto >> 8);
nhoff += PPPOE_SES_HLEN - 1;
} else {
ppp_proto = htons(ppp_proto);
nhoff += PPPOE_SES_HLEN;
}
>
> > > Should I use cpu_to_be16 on both 1 and 8. Is that what you mean?
> >
> > I can't see how cpu_to_be16() could help here. I was thinking of simply
> > using ntohs(hdr->proto).
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Intel-wired-lan] [RFC PATCH net-next v4 1/4] flow_dissector: Add PPPoE dissectors
@ 2022-07-13 13:54 ` Drewek, Wojciech
0 siblings, 0 replies; 28+ messages in thread
From: Drewek, Wojciech @ 2022-07-13 13:54 UTC (permalink / raw)
To: Guillaume Nault
Cc: simon.horman, kurt, paulb, edumazet, boris.sukholitko, paulus,
Brandeburg, Jesse, intel-wired-lan, kuba, zhangkaiheb, pablo,
baowen.zheng, jiri, komachi.yoshiki, jhs, mostrows,
xiyou.wangcong, pabeni, netdev, gustavoars, davem
> -----Original Message-----
> From: Drewek, Wojciech
> Sent: środa, 13 lipca 2022 09:59
> To: Guillaume Nault <gnault@redhat.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; kuba@kernel.org; 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>; mostrows@earthlink.net; paulus@samba.org
> Subject: RE: [RFC PATCH net-next v4 1/4] flow_dissector: Add PPPoE dissectors
>
>
>
> > -----Original Message-----
> > From: Guillaume Nault <gnault@redhat.com>
> > Sent: wtorek, 12 lipca 2022 19: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; kuba@kernel.org; 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>; mostrows@earthlink.net; paulus@samba.org
> > Subject: Re: [RFC PATCH net-next v4 1/4] flow_dissector: Add PPPoE dissectors
> >
> > On Mon, Jul 11, 2022 at 10:23:50AM +0000, Drewek, Wojciech wrote:
> > > > > diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
> > > > > index a4c6057c7097..af0d429b9a26 100644
> > > > > --- a/include/net/flow_dissector.h
> > > > > +++ b/include/net/flow_dissector.h
> > > > > @@ -261,6 +261,18 @@ 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
> > > > > + * @type: pppoe eth type
> > > > > + */
> > > > > +struct flow_dissector_key_pppoe {
> > > > > + __be16 session_id;
> > > > > + __be16 ppp_proto;
> > > > > + __be16 type;
> > > >
> > > > I don't understand the need for the new 'type' field.
> > >
> > > Let's say user want to add below filter with just protocol field:
> > > tc filter add dev ens6f0 ingress prio 1 protocol ppp_ses action drop
> > >
> > > cls_flower would set basic.n_proto to ETH_P_PPP_SES, then PPPoE packet
> > > arrives with ppp_proto = PPP_IP, which means that in __skb_flow_dissect basic.n_proto is going to
> > > be set to ETH_P_IP. We have a mismatch here cls_flower set basic.n_proto to ETH_P_PPP_SES and
> > > flow_dissector set it to ETH_P_IP. That's why in such example basic.n_proto has to be set to 0 (it works the same
> > > with vlans) and key_pppoe::type has to be used. In other words basic.n_proto can't be used for storing
> > > ETH_P_PPP_SES because it will store encapsulated protocol.
> > >
> > > We could also use it to match on ETH_P_PPP_DISC.
> >
> > Thanks for the explanation. That makes sense.
> >
> > > > > @@ -1214,26 +1250,60 @@ bool __skb_flow_dissect(const struct net *net,
> > > > > struct pppoe_hdr hdr;
> > > > > __be16 proto;
> > > > > } *hdr, _hdr;
> > > > > + __be16 ppp_proto;
> > > > > +
> > > > > hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr);
> > > > > if (!hdr) {
> > > > > fdret = FLOW_DISSECT_RET_OUT_BAD;
> > > > > break;
> > > > > }
> > > > >
> > > > > - nhoff += PPPOE_SES_HLEN;
> > > > > - switch (hdr->proto) {
> > > > > - case htons(PPP_IP):
> > > > > + if (!is_pppoe_ses_hdr_valid(hdr->hdr)) {
> > > > > + fdret = FLOW_DISSECT_RET_OUT_BAD;
> > > > > + break;
> > > > > + }
> > > > > +
> > > > > + /* least significant bit of the first byte
> > > > > + * indicates if protocol field was compressed
> > > > > + */
> > > > > + if (hdr->proto & 1) {
> > > > > + ppp_proto = hdr->proto << 8;
> > > >
> > > > This is little endian specific code. We can't make such assumptions.
> > >
> > > Both ppp_proto and hdr->prot are stored in __be16 so left shift by 8 bits
> > > should always be ok, am I right?
> >
> > Sorry, I don't understand. How could the test and the bit shift
> > operation give the correct result on a big endian machine?
> >
> > Let's say we handle an IPv4 packet and the PPP protocol field isn't
> > compressed. That is, protocol is 0x0021.
> > On a big endian machine 'hdr->proto & 1' is true and the bit shift sets
> > ppp_proto to 0x2100, while the code should have left the original value
> > untouched.
> >
>
> Ok, I see now, we'll fix it in the next version.
I think this should work with both LE and BE arch, what do you think Guillaume?
We don't want to spam so much with next versions so maybe it is better
to ask earlier.
u16 ppp_proto;
ppp_proto = ntohs(hdr->proto);
if (ppp_proto & 256) {
ppp_proto = htons(ppp_proto >> 8);
nhoff += PPPOE_SES_HLEN - 1;
} else {
ppp_proto = htons(ppp_proto);
nhoff += PPPOE_SES_HLEN;
}
>
> > > Should I use cpu_to_be16 on both 1 and 8. Is that what you mean?
> >
> > I can't see how cpu_to_be16() could help here. I was thinking of simply
> > using ntohs(hdr->proto).
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH net-next v4 1/4] flow_dissector: Add PPPoE dissectors
2022-07-13 13:54 ` [Intel-wired-lan] " Drewek, Wojciech
@ 2022-07-17 11:15 ` Guillaume Nault
-1 siblings, 0 replies; 28+ messages in thread
From: Guillaume Nault @ 2022-07-17 11:15 UTC (permalink / raw)
To: Drewek, Wojciech
Cc: Marcin Szycik, netdev, Nguyen, Anthony L, davem, xiyou.wangcong,
Brandeburg, Jesse, gustavoars, baowen.zheng, boris.sukholitko,
edumazet, kuba, jhs, jiri, kurt, pablo, pabeni, paulb,
simon.horman, komachi.yoshiki, zhangkaiheb, intel-wired-lan,
michal.swiatkowski, Lobakin, Alexandr, mostrows, paulus
On Wed, Jul 13, 2022 at 01:54:35PM +0000, Drewek, Wojciech wrote:
> I think this should work with both LE and BE arch, what do you think Guillaume?
> We don't want to spam so much with next versions so maybe it is better
> to ask earlier.
>
> u16 ppp_proto;
>
> ppp_proto = ntohs(hdr->proto);
> if (ppp_proto & 256) {
> ppp_proto = htons(ppp_proto >> 8);
> nhoff += PPPOE_SES_HLEN - 1;
> } else {
> ppp_proto = htons(ppp_proto);
> nhoff += PPPOE_SES_HLEN;
> }
Sorry for responding late. I was away this week (and will be next week
too) and have very sporadic (and slow) Internet connection and limitted
time for review. I saw you've sent a v5 with this code, I'll reply
there. Thanks for being so patient.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Intel-wired-lan] [RFC PATCH net-next v4 1/4] flow_dissector: Add PPPoE dissectors
@ 2022-07-17 11:15 ` Guillaume Nault
0 siblings, 0 replies; 28+ messages in thread
From: Guillaume Nault @ 2022-07-17 11:15 UTC (permalink / raw)
To: Drewek, Wojciech
Cc: simon.horman, kurt, paulb, edumazet, boris.sukholitko, paulus,
Brandeburg, Jesse, intel-wired-lan, kuba, zhangkaiheb, pablo,
baowen.zheng, jiri, komachi.yoshiki, jhs, mostrows,
xiyou.wangcong, pabeni, netdev, gustavoars, davem
On Wed, Jul 13, 2022 at 01:54:35PM +0000, Drewek, Wojciech wrote:
> I think this should work with both LE and BE arch, what do you think Guillaume?
> We don't want to spam so much with next versions so maybe it is better
> to ask earlier.
>
> u16 ppp_proto;
>
> ppp_proto = ntohs(hdr->proto);
> if (ppp_proto & 256) {
> ppp_proto = htons(ppp_proto >> 8);
> nhoff += PPPOE_SES_HLEN - 1;
> } else {
> ppp_proto = htons(ppp_proto);
> nhoff += PPPOE_SES_HLEN;
> }
Sorry for responding late. I was away this week (and will be next week
too) and have very sporadic (and slow) Internet connection and limitted
time for review. I saw you've sent a v5 with this code, I'll reply
there. Thanks for being so patient.
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2022-07-17 11:15 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-08 12:24 [RFC PATCH net-next v4 0/4] ice: PPPoE offload support Marcin Szycik
2022-07-08 12:24 ` [Intel-wired-lan] " Marcin Szycik
2022-07-08 12:24 ` [RFC PATCH net-next v4 1/4] flow_dissector: Add PPPoE dissectors Marcin Szycik
2022-07-08 12:24 ` [Intel-wired-lan] " Marcin Szycik
2022-07-08 19:05 ` Guillaume Nault
2022-07-08 19:05 ` [Intel-wired-lan] " Guillaume Nault
2022-07-11 10:23 ` Drewek, Wojciech
2022-07-11 10:23 ` Drewek, Wojciech
2022-07-12 17:20 ` Guillaume Nault
2022-07-12 17:20 ` [Intel-wired-lan] " Guillaume Nault
2022-07-13 7:58 ` Drewek, Wojciech
2022-07-13 7:58 ` [Intel-wired-lan] " Drewek, Wojciech
2022-07-13 13:54 ` Drewek, Wojciech
2022-07-13 13:54 ` [Intel-wired-lan] " Drewek, Wojciech
2022-07-17 11:15 ` Guillaume Nault
2022-07-17 11:15 ` [Intel-wired-lan] " Guillaume Nault
2022-07-08 12:24 ` [RFC PATCH net-next v4 2/4] net/sched: flower: Add PPPoE filter Marcin Szycik
2022-07-08 12:24 ` [Intel-wired-lan] " Marcin Szycik
2022-07-08 19:22 ` Guillaume Nault
2022-07-08 19:22 ` [Intel-wired-lan] " Guillaume Nault
2022-07-11 10:26 ` Drewek, Wojciech
2022-07-11 10:26 ` Drewek, Wojciech
2022-07-12 17:31 ` Guillaume Nault
2022-07-12 17:31 ` [Intel-wired-lan] " Guillaume Nault
2022-07-08 12:24 ` [RFC PATCH net-next v4 3/4] flow_offload: Introduce flow_match_pppoe Marcin Szycik
2022-07-08 12:24 ` [Intel-wired-lan] " Marcin Szycik
2022-07-08 12:24 ` [RFC PATCH net-next v4 4/4] ice: Add support for PPPoE hardware offload Marcin Szycik
2022-07-08 12:24 ` [Intel-wired-lan] " Marcin Szycik
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.