Skip to content

Commit b89d06c

Browse files
davem330davem330
committed
Merge branch 'use-tc_cls_can_offload_and_chain0-throughout-the-drivers'
Jakub Kicinski says: ==================== use tc_cls_can_offload_and_chain0() throughout the drivers This set makes all drivers use a new tc_cls_can_offload_and_chain0() helper which will set extack in case TC hw offload flag is disabled. I chose to keep the new helper which also looks at the chain but renamed it more appropriately. The rationale being that most drivers don't accept chains other than 0 and since we have to pass extack to the helper we can as well pass the entire struct tc_cls_common_offload and perform the most common checks. This code makes the assumption that type_data in the callback can be interpreted as struct tc_cls_common_offload, i.e. the real offload structure has common part as the first member. This allows us to make the check once for all classifier types if driver supports more than one. v1: - drop the type validation in nfp and netdevsim. v2: - reorder checks in patch 1; - split other changes from patch 1; - add the i40e patch in; - add one more test case - for chain 0 extack. ==================== Signed-off-by: David S. Miller <[email protected]>
2 parents fdd6d77 + baf6a07 commit b89d06c

File tree

14 files changed

+72
-47
lines changed

14 files changed

+72
-47
lines changed

‎drivers/net/ethernet/broadcom/bnxt/bnxt.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7778,7 +7778,8 @@ static int bnxt_setup_tc_block_cb(enum tc_setup_type type, void *type_data,
77787778
{
77797779
struct bnxt *bp = cb_priv;
77807780

7781-
if (!bnxt_tc_flower_enabled(bp) || !tc_can_offload(bp->dev))
7781+
if (!bnxt_tc_flower_enabled(bp) ||
7782+
!tc_cls_can_offload_and_chain0(bp->dev, type_data))
77827783
return -EOPNOTSUPP;
77837784

77847785
switch (type) {

‎drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1474,9 +1474,6 @@ int bnxt_tc_setup_flower(struct bnxt *bp, u16 src_fid,
14741474
{
14751475
int rc = 0;
14761476

1477-
if (cls_flower->common.chain_index)
1478-
return -EOPNOTSUPP;
1479-
14801477
switch (cls_flower->command) {
14811478
case TC_CLSFLOWER_REPLACE:
14821479
rc = bnxt_tc_add_flow(bp, src_fid, cls_flower);

‎drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,8 @@ static int bnxt_vf_rep_setup_tc_block_cb(enum tc_setup_type type,
124124
struct bnxt *bp = vf_rep->bp;
125125
int vf_fid = bp->pf.vf[vf_rep->vf_idx].fw_fid;
126126

127-
if (!bnxt_tc_flower_enabled(vf_rep->bp) || !tc_can_offload(bp->dev))
127+
if (!bnxt_tc_flower_enabled(vf_rep->bp) ||
128+
!tc_cls_can_offload_and_chain0(bp->dev, type_data))
128129
return -EOPNOTSUPP;
129130

130131
switch (type) {

‎drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2928,9 +2928,6 @@ static int cxgb_set_tx_maxrate(struct net_device *dev, int index, u32 rate)
29282928
static int cxgb_setup_tc_flower(struct net_device *dev,
29292929
struct tc_cls_flower_offload *cls_flower)
29302930
{
2931-
if (cls_flower->common.chain_index)
2932-
return -EOPNOTSUPP;
2933-
29342931
switch (cls_flower->command) {
29352932
case TC_CLSFLOWER_REPLACE:
29362933
return cxgb4_tc_flower_replace(dev, cls_flower);
@@ -2946,9 +2943,6 @@ static int cxgb_setup_tc_flower(struct net_device *dev,
29462943
static int cxgb_setup_tc_cls_u32(struct net_device *dev,
29472944
struct tc_cls_u32_offload *cls_u32)
29482945
{
2949-
if (cls_u32->common.chain_index)
2950-
return -EOPNOTSUPP;
2951-
29522946
switch (cls_u32->command) {
29532947
case TC_CLSU32_NEW_KNODE:
29542948
case TC_CLSU32_REPLACE_KNODE:
@@ -2974,7 +2968,7 @@ static int cxgb_setup_tc_block_cb(enum tc_setup_type type, void *type_data,
29742968
return -EINVAL;
29752969
}
29762970

2977-
if (!tc_can_offload(dev))
2971+
if (!tc_cls_can_offload_and_chain0(dev, type_data))
29782972
return -EOPNOTSUPP;
29792973

29802974
switch (type) {

‎drivers/net/ethernet/intel/i40e/i40e_main.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7437,11 +7437,6 @@ static int i40e_setup_tc_cls_flower(struct i40e_netdev_priv *np,
74377437
{
74387438
struct i40e_vsi *vsi = np->vsi;
74397439

7440-
if (!tc_can_offload(vsi->netdev))
7441-
return -EOPNOTSUPP;
7442-
if (cls_flower->common.chain_index)
7443-
return -EOPNOTSUPP;
7444-
74457440
switch (cls_flower->command) {
74467441
case TC_CLSFLOWER_REPLACE:
74477442
return i40e_configure_clsflower(vsi, cls_flower);
@@ -7459,6 +7454,9 @@ static int i40e_setup_tc_block_cb(enum tc_setup_type type, void *type_data,
74597454
{
74607455
struct i40e_netdev_priv *np = cb_priv;
74617456

7457+
if (!tc_cls_can_offload_and_chain0(np->vsi->netdev, type_data))
7458+
return -EOPNOTSUPP;
7459+
74627460
switch (type) {
74637461
case TC_SETUP_CLSFLOWER:
74647462
return i40e_setup_tc_cls_flower(np, type_data);

‎drivers/net/ethernet/intel/ixgbe/ixgbe_main.c

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9303,9 +9303,6 @@ static int ixgbe_configure_clsu32(struct ixgbe_adapter *adapter,
93039303
static int ixgbe_setup_tc_cls_u32(struct ixgbe_adapter *adapter,
93049304
struct tc_cls_u32_offload *cls_u32)
93059305
{
9306-
if (cls_u32->common.chain_index)
9307-
return -EOPNOTSUPP;
9308-
93099306
switch (cls_u32->command) {
93109307
case TC_CLSU32_NEW_KNODE:
93119308
case TC_CLSU32_REPLACE_KNODE:
@@ -9327,7 +9324,7 @@ static int ixgbe_setup_tc_block_cb(enum tc_setup_type type, void *type_data,
93279324
{
93289325
struct ixgbe_adapter *adapter = cb_priv;
93299326

9330-
if (!tc_can_offload(adapter->netdev))
9327+
if (!tc_cls_can_offload_and_chain0(adapter->netdev, type_data))
93319328
return -EOPNOTSUPP;
93329329

93339330
switch (type) {

‎drivers/net/ethernet/mellanox/mlx5/core/en_main.c

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2944,9 +2944,6 @@ static int mlx5e_setup_tc_mqprio(struct net_device *netdev,
29442944
static int mlx5e_setup_tc_cls_flower(struct mlx5e_priv *priv,
29452945
struct tc_cls_flower_offload *cls_flower)
29462946
{
2947-
if (cls_flower->common.chain_index)
2948-
return -EOPNOTSUPP;
2949-
29502947
switch (cls_flower->command) {
29512948
case TC_CLSFLOWER_REPLACE:
29522949
return mlx5e_configure_flower(priv, cls_flower);
@@ -2964,7 +2961,7 @@ int mlx5e_setup_tc_block_cb(enum tc_setup_type type, void *type_data,
29642961
{
29652962
struct mlx5e_priv *priv = cb_priv;
29662963

2967-
if (!tc_can_offload(priv->netdev))
2964+
if (!tc_cls_can_offload_and_chain0(priv->netdev, type_data))
29682965
return -EOPNOTSUPP;
29692966

29702967
switch (type) {

‎drivers/net/ethernet/mellanox/mlx5/core/en_rep.c

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -719,9 +719,6 @@ static int
719719
mlx5e_rep_setup_tc_cls_flower(struct mlx5e_priv *priv,
720720
struct tc_cls_flower_offload *cls_flower)
721721
{
722-
if (cls_flower->common.chain_index)
723-
return -EOPNOTSUPP;
724-
725722
switch (cls_flower->command) {
726723
case TC_CLSFLOWER_REPLACE:
727724
return mlx5e_configure_flower(priv, cls_flower);
@@ -739,7 +736,7 @@ static int mlx5e_rep_setup_tc_cb(enum tc_setup_type type, void *type_data,
739736
{
740737
struct mlx5e_priv *priv = cb_priv;
741738

742-
if (!tc_can_offload(priv->netdev))
739+
if (!tc_cls_can_offload_and_chain0(priv->netdev, type_data))
743740
return -EOPNOTSUPP;
744741

745742
switch (type) {

‎drivers/net/ethernet/mellanox/mlxsw/spectrum.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1738,9 +1738,6 @@ static int mlxsw_sp_setup_tc_cls_matchall(struct mlxsw_sp_port *mlxsw_sp_port,
17381738
struct tc_cls_matchall_offload *f,
17391739
bool ingress)
17401740
{
1741-
if (f->common.chain_index)
1742-
return -EOPNOTSUPP;
1743-
17441741
switch (f->command) {
17451742
case TC_CLSMATCHALL_REPLACE:
17461743
return mlxsw_sp_port_add_cls_matchall(mlxsw_sp_port, f,
@@ -1780,7 +1777,8 @@ static int mlxsw_sp_setup_tc_block_cb_matchall(enum tc_setup_type type,
17801777

17811778
switch (type) {
17821779
case TC_SETUP_CLSMATCHALL:
1783-
if (!tc_can_offload(mlxsw_sp_port->dev))
1780+
if (!tc_cls_can_offload_and_chain0(mlxsw_sp_port->dev,
1781+
type_data))
17841782
return -EOPNOTSUPP;
17851783

17861784
return mlxsw_sp_setup_tc_cls_matchall(mlxsw_sp_port, type_data,

‎drivers/net/ethernet/netronome/nfp/bpf/main.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ static int nfp_bpf_setup_tc_block_cb(enum tc_setup_type type,
130130
"only offload of BPF classifiers supported");
131131
return -EOPNOTSUPP;
132132
}
133-
if (!tc_can_offload_extack(nn->dp.netdev, cls_bpf->common.extack))
133+
if (!tc_cls_can_offload_and_chain0(nn->dp.netdev, &cls_bpf->common))
134134
return -EOPNOTSUPP;
135135
if (!nfp_net_ebpf_capable(nn)) {
136136
NL_SET_ERR_MSG_MOD(cls_bpf->common.extack,
@@ -142,8 +142,6 @@ static int nfp_bpf_setup_tc_block_cb(enum tc_setup_type type,
142142
"only ETH_P_ALL supported as filter protocol");
143143
return -EOPNOTSUPP;
144144
}
145-
if (cls_bpf->common.chain_index)
146-
return -EOPNOTSUPP;
147145

148146
/* Only support TC direct action */
149147
if (!cls_bpf->exts_integrated ||

‎drivers/net/ethernet/netronome/nfp/flower/offload.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -483,8 +483,7 @@ static int
483483
nfp_flower_repr_offload(struct nfp_app *app, struct net_device *netdev,
484484
struct tc_cls_flower_offload *flower, bool egress)
485485
{
486-
if (!eth_proto_is_802_3(flower->common.protocol) ||
487-
flower->common.chain_index)
486+
if (!eth_proto_is_802_3(flower->common.protocol))
488487
return -EOPNOTSUPP;
489488

490489
switch (flower->command) {
@@ -504,7 +503,7 @@ int nfp_flower_setup_tc_egress_cb(enum tc_setup_type type, void *type_data,
504503
{
505504
struct nfp_repr *repr = cb_priv;
506505

507-
if (!tc_can_offload(repr->netdev))
506+
if (!tc_cls_can_offload_and_chain0(repr->netdev, type_data))
508507
return -EOPNOTSUPP;
509508

510509
switch (type) {
@@ -521,7 +520,7 @@ static int nfp_flower_setup_tc_block_cb(enum tc_setup_type type,
521520
{
522521
struct nfp_repr *repr = cb_priv;
523522

524-
if (!tc_can_offload(repr->netdev))
523+
if (!tc_cls_can_offload_and_chain0(repr->netdev, type_data))
525524
return -EOPNOTSUPP;
526525

527526
switch (type) {

‎drivers/net/netdevsim/bpf.c

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ int nsim_bpf_setup_tc_block_cb(enum tc_setup_type type,
135135
return -EOPNOTSUPP;
136136
}
137137

138-
if (!tc_can_offload_extack(ns->netdev, cls_bpf->common.extack))
138+
if (!tc_cls_can_offload_and_chain0(ns->netdev, &cls_bpf->common))
139139
return -EOPNOTSUPP;
140140

141141
if (cls_bpf->common.protocol != htons(ETH_P_ALL)) {
@@ -144,9 +144,6 @@ int nsim_bpf_setup_tc_block_cb(enum tc_setup_type type,
144144
return -EOPNOTSUPP;
145145
}
146146

147-
if (cls_bpf->common.chain_index)
148-
return -EOPNOTSUPP;
149-
150147
if (!ns->bpf_tc_accept) {
151148
NSIM_EA(cls_bpf->common.extack,
152149
"netdevsim configured to reject BPF TC offload");

‎include/net/pkt_cls.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -656,6 +656,20 @@ static inline bool tc_can_offload_extack(const struct net_device *dev,
656656
return can;
657657
}
658658

659+
static inline bool
660+
tc_cls_can_offload_and_chain0(const struct net_device *dev,
661+
struct tc_cls_common_offload *common)
662+
{
663+
if (!tc_can_offload_extack(dev, common->extack))
664+
return false;
665+
if (common->chain_index) {
666+
NL_SET_ERR_MSG(common->extack,
667+
"Driver supports only offload of chain 0");
668+
return false;
669+
}
670+
return true;
671+
}
672+
659673
static inline bool tc_skip_hw(u32 flags)
660674
{
661675
return (flags & TCA_CLS_FLAGS_SKIP_HW) ? true : false;

‎tools/testing/selftests/bpf/test_offload.py

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -430,21 +430,23 @@ def tc_show_ingress(self, expected=None):
430430
return filters
431431

432432
def cls_filter_op(self, op, qdisc="ingress", prio=None, handle=None,
433-
cls="", params="",
433+
chain=None, cls="", params="",
434434
fail=True, include_stderr=False):
435435
spec = ""
436436
if prio is not None:
437437
spec += " prio %d" % (prio)
438438
if handle:
439439
spec += " handle %s" % (handle)
440+
if chain is not None:
441+
spec += " chain %d" % (chain)
440442

441443
return tc("filter {op} dev {dev} {qdisc} {spec} {cls} {params}"\
442444
.format(op=op, dev=self['ifname'], qdisc=qdisc, spec=spec,
443445
cls=cls, params=params),
444446
fail=fail, include_stderr=include_stderr)
445447

446448
def cls_bpf_add_filter(self, bpf, op="add", prio=None, handle=None,
447-
da=False, verbose=False,
449+
chain=None, da=False, verbose=False,
448450
skip_sw=False, skip_hw=False,
449451
fail=True, include_stderr=False):
450452
cls = "bpf " + bpf
@@ -460,7 +462,7 @@ def cls_bpf_add_filter(self, bpf, op="add", prio=None, handle=None,
460462
params += " skip_hw"
461463

462464
return self.cls_filter_op(op=op, prio=prio, handle=handle, cls=cls,
463-
params=params,
465+
chain=chain, params=params,
464466
fail=fail, include_stderr=include_stderr)
465467

466468
def set_ethtool_tc_offloads(self, enable, fail=True):
@@ -543,13 +545,29 @@ def check_extack(output, reference, args):
543545
def check_extack_nsim(output, reference, args):
544546
check_extack(output, "Error: netdevsim: " + reference, args)
545547

548+
def check_no_extack(res, needle):
549+
fail((res[1] + res[2]).count(needle) or (res[1] + res[2]).count("Warning:"),
550+
"Found '%s' in command output, leaky extack?" % (needle))
551+
546552
def check_verifier_log(output, reference):
547553
lines = output.split("\n")
548554
for l in reversed(lines):
549555
if l == reference:
550556
return
551557
fail(True, "Missing or incorrect message from netdevsim in verifier log")
552558

559+
def test_spurios_extack(sim, obj, skip_hw, needle):
560+
res = sim.cls_bpf_add_filter(obj, prio=1, handle=1, skip_hw=skip_hw,
561+
include_stderr=True)
562+
check_no_extack(res, needle)
563+
res = sim.cls_bpf_add_filter(obj, op="replace", prio=1, handle=1,
564+
skip_hw=skip_hw, include_stderr=True)
565+
check_no_extack(res, needle)
566+
res = sim.cls_filter_op(op="delete", prio=1, handle=1, cls="bpf",
567+
include_stderr=True)
568+
check_no_extack(res, needle)
569+
570+
553571
# Parse command line
554572
parser = argparse.ArgumentParser()
555573
parser.add_argument("--log", help="output verbose log to given file")
@@ -663,6 +681,14 @@ def check_verifier_log(output, reference):
663681
args)
664682
sim.wait_for_flush()
665683

684+
start_test("Test non-0 chain offload...")
685+
ret, _, err = sim.cls_bpf_add_filter(obj, chain=1, prio=1, handle=1,
686+
skip_sw=True,
687+
fail=False, include_stderr=True)
688+
fail(ret == 0, "Offloaded a filter to chain other than 0")
689+
check_extack(err, "Error: Driver supports only offload of chain 0.", args)
690+
sim.tc_flush_filters()
691+
666692
start_test("Test TC replace...")
667693
sim.cls_bpf_add_filter(obj, prio=1, handle=1)
668694
sim.cls_bpf_add_filter(obj, op="replace", prio=1, handle=1)
@@ -687,6 +713,17 @@ def check_verifier_log(output, reference):
687713
(j))
688714
sim.cls_filter_op(op="delete", prio=1, handle=1, cls="bpf")
689715

716+
start_test("Test spurious extack from the driver...")
717+
test_spurios_extack(sim, obj, False, "netdevsim")
718+
test_spurios_extack(sim, obj, True, "netdevsim")
719+
720+
sim.set_ethtool_tc_offloads(False)
721+
722+
test_spurios_extack(sim, obj, False, "TC offload is disabled")
723+
test_spurios_extack(sim, obj, True, "TC offload is disabled")
724+
725+
sim.set_ethtool_tc_offloads(True)
726+
690727
sim.tc_flush_filters()
691728

692729
start_test("Test TC offloads work...")

0 commit comments

Comments
 (0)