Add a set-dp command to ovs-dpctl for the kernel's datapath SET command, and a test that uses it. The test clears the features of a fresh datapath, sets the dispatch bit with a valid pid list, then sends a set that would set the same bit with an empty per-cpu-pids. The request has to fail with EINVAL, and the datapath has to keep reporting the features it had. A second round covers the other direction: with the recirc bit set, a failing request that would drop it has to leave it in place. That round needs CONFIG_NET_TC_SKB_EXT. The selftest config fragment did not select it, so a kernel built from the fragment rejected the recirc bit with EOPNOTSUPP and the round could not run. Select the key and the classful action support it depends on, and skip the test with the kselftest skip code when the kernel still reports EOPNOTSUPP. Any other failure of that request fails the test rather than skipping it. set-dp sends the features it is given. When --features is left out it sends the features the datapath reports, so a request that only changes the pid list does not clear the other bits. Both rounds fail without the fix in the previous patch, which reports the features the failed request asked for instead of the old ones. Signed-off-by: Minxi Hou --- .../testing/selftests/net/openvswitch/config | 2 + .../selftests/net/openvswitch/openvswitch.sh | 67 +++++++++++++++++++ .../selftests/net/openvswitch/ovs-dpctl.py | 53 +++++++++++++++ 3 files changed, 122 insertions(+) diff --git a/tools/testing/selftests/net/openvswitch/config b/tools/testing/selftests/net/openvswitch/config index 15685d242813..d93eac0f08f7 100644 --- a/tools/testing/selftests/net/openvswitch/config +++ b/tools/testing/selftests/net/openvswitch/config @@ -2,6 +2,8 @@ CONFIG_GENEVE=m CONFIG_INET_DIAG=y CONFIG_IP_SCTP=y CONFIG_IPV6=y +CONFIG_NET_CLS_ACT=y +CONFIG_NET_TC_SKB_EXT=y CONFIG_NETFILTER=y CONFIG_NETFILTER_ADVANCED=y CONFIG_NET_IPGRE=m diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh index 21444657a040..e2aba1e40d55 100755 --- a/tools/testing/selftests/net/openvswitch/openvswitch.sh +++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh @@ -24,6 +24,7 @@ tests=" nat_connect_v4 ip4-nat-xon: Basic ipv4 tcp connection via NAT nat_related_v4 ip4-nat-related: ICMP related matches work with SNAT netlink_checks ovsnl: validate netlink attrs and settings + dp_features ovsdp: keep the set features upcall_interfaces ovs: test the upcall interfaces tunnel_metadata ovs: test extraction of tunnel metadata drop_reason drop: test drop reasons are emitted @@ -1409,6 +1410,72 @@ test_netlink_checks () { return 0 } +test_dp_features() { + dp_features_check() { + python3 $ovs_base/ovs-dpctl.py show "$1" | + grep -q "features: 0x$2\$" + } + + dp_features_set() { + python3 $ovs_base/ovs-dpctl.py set-dp "$@" + } + + sbx_add "test_dp_features" || return $? + ovs_add_dp "test_dp_features" dpfeat || return 1 + + # 8 is OVS_DP_F_DISPATCH_UPCALL_PER_CPU, the bit that makes the + # kernel read per-cpu-pids. + info "clear the datapath features" + ovs_sbx "test_dp_features" dp_features_set dpfeat --features 0 \ + || return 1 + ovs_sbx "test_dp_features" dp_features_check dpfeat 0 \ + || { info "features were not cleared"; return 1; } + + info "a set with the dispatch bit and pids is accepted" + ovs_sbx "test_dp_features" dp_features_set dpfeat --features 8 \ + --per-cpu-pids 1,2 || return 1 + ovs_sbx "test_dp_features" dp_features_check dpfeat 8 \ + || { info "the accepted set was not applied"; return 1; } + + ovs_sbx "test_dp_features" dp_features_set dpfeat --features 0 \ + || return 1 + + info "a set with an empty per-cpu-pids is rejected" + ovs_sbx "test_dp_features" dp_features_set dpfeat --features 8 \ + --per-cpu-pids "" &> /dev/null + [ $? -eq 22 ] \ + || { info "the empty per-cpu-pids set did not fail with EINVAL"; \ + return 1; } + + info "the rejected set keeps the features it had" + ovs_sbx "test_dp_features" dp_features_check dpfeat 0 \ + || { info "the rejected set was recorded"; return 1; } + + # 4 is OVS_DP_F_TC_RECIRC_SHARING, which the kernel only accepts when + # the tc skb extension is compiled in. A rejected set that drops the + # bit must leave the key alone, so the features have to stay as they + # were. + ovs_sbx "test_dp_features" dp_features_set dpfeat --features 4 \ + &> /dev/null + rc=$? + if [ $rc -eq 95 ]; then + info "tc skb extension is not compiled in, skipping recirc" + return $ksft_skip + elif [ $rc -ne 0 ]; then + info "the recirc set failed with $rc" + return 1 + fi + info "a rejected set does not drop the recirc bit" + ovs_sbx "test_dp_features" dp_features_set dpfeat --features 8 \ + --per-cpu-pids "" &> /dev/null + [ $? -eq 22 ] \ + || { info "the clearing set did not fail with EINVAL"; return 1; } + ovs_sbx "test_dp_features" dp_features_check dpfeat 4 \ + || { info "the rejected set dropped the recirc bit"; return 1; } + + return 0 +} + test_upcall_interfaces() { sbx_add "test_upcall_interfaces" || return 1 diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py index 1615843c225e..1b02d383e49e 100644 --- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py +++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py @@ -2301,6 +2301,24 @@ class OvsDatapath(GenericNetlinkSocket): return reply + def set(self, dpname, ifindex=0, features=None, per_cpu_pids=None): + msg = OvsDatapath.dp_cmd_msg() + msg["cmd"] = OVS_DP_CMD_SET + msg["version"] = OVS_DATAPATH_VERSION + msg["reserved"] = 0 + msg["dpifindex"] = ifindex + msg["attrs"].append(["OVS_DP_ATTR_NAME", dpname]) + if features is not None: + msg["attrs"].append(["OVS_DP_ATTR_USER_FEATURES", features]) + if per_cpu_pids is not None: + msg["attrs"].append(["OVS_DP_ATTR_PER_CPU_PIDS", per_cpu_pids]) + + reply = self.nlm_request( + msg, msg_type=self.prid, msg_flags=NLM_F_REQUEST | NLM_F_ACK + ) + + return reply[0] + def create( self, dpname, shouldUpcall=False, versionStr=None, p=OvsPacket() ): @@ -2961,6 +2979,23 @@ def main(argv): deldpcmd = subparsers.add_parser("del-dp") deldpcmd.add_argument("deldp", help="Datapath Name") + setdpcmd = subparsers.add_parser("set-dp") + setdpcmd.add_argument("setdp", help="Datapath Name") + setdpcmd.add_argument( + "-f", + "--features", + type=lambda x: int(x, 0), + default=None, + help="Datapath user features, defaults to the current value", + ) + setdpcmd.add_argument( + "-p", + "--per-cpu-pids", + type=str, + default=None, + help="Comma separated upcall pids, an empty string sends none", + ) + addifcmd = subparsers.add_parser("add-if") addifcmd.add_argument("dpname", help="Datapath Name") addifcmd.add_argument("addif", help="Interface name for adding") @@ -3057,6 +3092,24 @@ def main(argv): ovspk.upcall_handler(ovsflow) elif hasattr(args, "deldp"): ovsdp.destroy(args.deldp) + elif hasattr(args, "setdp"): + rep = ovsdp.info(args.setdp, 0) + if rep is None: + print("DP '%s' not found." % args.setdp) + return 1 + pids = None + if args.per_cpu_pids is not None: + pids = [int(x) for x in args.per_cpu_pids.split(",") + if x != ""] + features = args.features + if features is None: + features = rep.get_attr("OVS_DP_ATTR_USER_FEATURES") + try: + ovsdp.set(args.setdp, rep["dpifindex"], features, pids) + except NetlinkError as ne: + print("set failed: %s" % ne) + return ne.code + print("DP '%s' updated" % args.setdp) elif hasattr(args, "addif"): rep = ovsdp.info(args.dpname, 0) if rep is None: -- 2.56.0