| Message ID | 20240711233238.1038670-4-amorenoz@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce local sampling with NXAST_SAMPLE action. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On 12 Jul 2024, at 1:32, Adrian Moreno wrote: > Only kernel datapath supports this action so add a function in dpif.c > that checks for that. > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> This patch looks good to me. One small comment/nit below. The rest looks good. Acked-by: Eelco Chaudron <echaudro@redhat.com> > --- > lib/dpif.c | 7 +++++++ > lib/dpif.h | 1 + > ofproto/ofproto-dpif.c | 46 ++++++++++++++++++++++++++++++++++++++++++ > ofproto/ofproto-dpif.h | 6 +++++- > vswitchd/vswitch.xml | 5 +++++ > 5 files changed, 64 insertions(+), 1 deletion(-) > > diff --git a/lib/dpif.c b/lib/dpif.c > index 94db4630e..ab633fd27 100644 > --- a/lib/dpif.c > +++ b/lib/dpif.c > @@ -1953,6 +1953,13 @@ dpif_supports_lb_output_action(const struct dpif *dpif) > return dpif_is_netdev(dpif); > } > > +bool > +dpif_may_support_psample(const struct dpif *dpif) > +{ > + /* Userspace datapath does not support this action. */ > + return !dpif_is_netdev(dpif); > +} > + > /* Meters */ > void > dpif_meter_get_features(const struct dpif *dpif, > diff --git a/lib/dpif.h b/lib/dpif.h > index a764e8a59..6bef7d5b3 100644 > --- a/lib/dpif.h > +++ b/lib/dpif.h > @@ -941,6 +941,7 @@ int dpif_get_pmds_for_port(const struct dpif * dpif, odp_port_t port_no, > char *dpif_get_dp_version(const struct dpif *); > bool dpif_supports_tnl_push_pop(const struct dpif *); > bool dpif_may_support_explicit_drop_action(const struct dpif *); > +bool dpif_may_support_psample(const struct dpif *); > bool dpif_synced_dp_layers(struct dpif *); > > /* Log functions. */ > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > index 1405d614e..41a5e989d 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -873,6 +873,12 @@ ovs_lb_output_action_supported(struct ofproto_dpif *ofproto) > return ofproto->backer->rt_support.lb_output_action; > } > > +bool > +ovs_psample_supported(struct ofproto_dpif *ofproto) > +{ > + return ofproto->backer->rt_support.psample; > +} > + > /* Tests whether 'backer''s datapath supports recirculation. Only newer > * datapaths support OVS_KEY_ATTR_RECIRC_ID in keys. We need to disable some > * features on older datapaths that don't support this feature. > @@ -1609,6 +1615,44 @@ check_add_mpls(struct dpif_backer *backer) > return supported; > } > > +/* Tests whether 'backer''s datapath supports the OVS_ACTION_ATTR_PSAMPLE > + * action. */ > +static bool > +check_psample(struct dpif_backer *backer) > +{ > + uint8_t cookie[OVS_PSAMPLE_COOKIE_MAX_SIZE]; > + struct odputil_keybuf keybuf; > + struct ofpbuf actions; > + struct ofpbuf key; > + bool supported; > + > + /* Intentionally bogus dl_type. */ > + struct flow flow = { > + .dl_type = CONSTANT_HTONS(0x1234), > + }; > + struct odp_flow_key_parms odp_parms = { > + .flow = &flow, > + .probe = true, > + }; > + > + ofpbuf_use_stack(&key, &keybuf, sizeof keybuf); > + odp_flow_key_from_flow(&odp_parms, &key); > + ofpbuf_init(&actions, 32); > + > + /* Generate a random max-size cookie. */ > + random_bytes(cookie, sizeof cookie); > + > + odp_put_psample_action(&actions, 10, cookie, sizeof cookie); > + > + supported = dpif_may_support_psample(backer->dpif) && > + dpif_probe_feature(backer->dpif, "psample", &key, &actions, NULL); > + > + ofpbuf_uninit(&actions); > + VLOG_INFO("%s: Datapath %s psample action", dpif_name(backer->dpif), > + supported ? "supports" : "does not support"); > + return supported; > +} > + > #define CHECK_FEATURE__(NAME, SUPPORT, FIELD, VALUE, ETHTYPE) \ > static bool \ > check_##NAME(struct dpif_backer *backer) \ > @@ -1698,6 +1742,7 @@ check_support(struct dpif_backer *backer) > dpif_supports_lb_output_action(backer->dpif); > backer->rt_support.ct_zero_snat = dpif_supports_ct_zero_snat(backer); > backer->rt_support.add_mpls = check_add_mpls(backer); > + backer->rt_support.psample = check_psample(backer); > > /* Flow fields. */ > backer->rt_support.odp.ct_state = check_ct_state(backer); > @@ -5821,6 +5866,7 @@ get_datapath_cap(const char *datapath_type, struct smap *cap) > smap_add(cap, "lb_output_action", s->lb_output_action ? "true" : "false"); > smap_add(cap, "ct_zero_snat", s->ct_zero_snat ? "true" : "false"); > smap_add(cap, "add_mpls", s->add_mpls ? "true" : "false"); > + smap_add(cap, "psample", s->psample ? "true" : "false"); > > /* The ct_tuple_flush is implemented on dpif level, so it is supported > * for all backers. */ > diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h > index d33f73df8..ea18ccedf 100644 > --- a/ofproto/ofproto-dpif.h > +++ b/ofproto/ofproto-dpif.h > @@ -213,7 +213,10 @@ struct group_dpif *group_dpif_lookup(struct ofproto_dpif *, > DPIF_SUPPORT_FIELD(bool, ct_zero_snat, "Conntrack all-zero IP SNAT") \ > \ > /* True if the datapath supports add_mpls action. */ \ > - DPIF_SUPPORT_FIELD(bool, add_mpls, "MPLS Label add") > + DPIF_SUPPORT_FIELD(bool, add_mpls, "MPLS Label add") \ > + \ > + /* True if the datapath supports psample action. */ \ > + DPIF_SUPPORT_FIELD(bool, psample, "psample") > > > /* Stores the various features which the corresponding backer supports. */ > @@ -411,5 +414,6 @@ bool ofproto_dpif_ct_zone_timeout_policy_get_name( > uint8_t nw_proto, char **tp_name, bool *unwildcard); > > bool ovs_explicit_drop_action_supported(struct ofproto_dpif *); > +bool ovs_psample_supported(struct ofproto_dpif *); > > #endif /* ofproto-dpif.h */ > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml > index e3afb78a4..c6eb598d3 100644 > --- a/vswitchd/vswitch.xml > +++ b/vswitchd/vswitch.xml > @@ -6511,6 +6511,11 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \ > called <code>NXT_CT_FLUSH</code>. The <code>NXT_CT_FLUSH</code> > extensions allows to flush CT entries based on specified parameters. > </column> > + <column name="capabilities" key="psample" > + type='{"type": "boolean"}'> > + True if the datapath supports OVS_ACTION_ATTR_PSAMPLE. If false, > + local sampling will not be supported. nit: Remember to change this if we add userspace support, or maybe make it more clear with something like: If false, local sampling will not be supported with the Linux kernel datapath. > + </column> > </group> > > <column name="ct_zone_default_limit"> > -- > 2.45.2
diff --git a/lib/dpif.c b/lib/dpif.c index 94db4630e..ab633fd27 100644 --- a/lib/dpif.c +++ b/lib/dpif.c @@ -1953,6 +1953,13 @@ dpif_supports_lb_output_action(const struct dpif *dpif) return dpif_is_netdev(dpif); } +bool +dpif_may_support_psample(const struct dpif *dpif) +{ + /* Userspace datapath does not support this action. */ + return !dpif_is_netdev(dpif); +} + /* Meters */ void dpif_meter_get_features(const struct dpif *dpif, diff --git a/lib/dpif.h b/lib/dpif.h index a764e8a59..6bef7d5b3 100644 --- a/lib/dpif.h +++ b/lib/dpif.h @@ -941,6 +941,7 @@ int dpif_get_pmds_for_port(const struct dpif * dpif, odp_port_t port_no, char *dpif_get_dp_version(const struct dpif *); bool dpif_supports_tnl_push_pop(const struct dpif *); bool dpif_may_support_explicit_drop_action(const struct dpif *); +bool dpif_may_support_psample(const struct dpif *); bool dpif_synced_dp_layers(struct dpif *); /* Log functions. */ diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 1405d614e..41a5e989d 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -873,6 +873,12 @@ ovs_lb_output_action_supported(struct ofproto_dpif *ofproto) return ofproto->backer->rt_support.lb_output_action; } +bool +ovs_psample_supported(struct ofproto_dpif *ofproto) +{ + return ofproto->backer->rt_support.psample; +} + /* Tests whether 'backer''s datapath supports recirculation. Only newer * datapaths support OVS_KEY_ATTR_RECIRC_ID in keys. We need to disable some * features on older datapaths that don't support this feature. @@ -1609,6 +1615,44 @@ check_add_mpls(struct dpif_backer *backer) return supported; } +/* Tests whether 'backer''s datapath supports the OVS_ACTION_ATTR_PSAMPLE + * action. */ +static bool +check_psample(struct dpif_backer *backer) +{ + uint8_t cookie[OVS_PSAMPLE_COOKIE_MAX_SIZE]; + struct odputil_keybuf keybuf; + struct ofpbuf actions; + struct ofpbuf key; + bool supported; + + /* Intentionally bogus dl_type. */ + struct flow flow = { + .dl_type = CONSTANT_HTONS(0x1234), + }; + struct odp_flow_key_parms odp_parms = { + .flow = &flow, + .probe = true, + }; + + ofpbuf_use_stack(&key, &keybuf, sizeof keybuf); + odp_flow_key_from_flow(&odp_parms, &key); + ofpbuf_init(&actions, 32); + + /* Generate a random max-size cookie. */ + random_bytes(cookie, sizeof cookie); + + odp_put_psample_action(&actions, 10, cookie, sizeof cookie); + + supported = dpif_may_support_psample(backer->dpif) && + dpif_probe_feature(backer->dpif, "psample", &key, &actions, NULL); + + ofpbuf_uninit(&actions); + VLOG_INFO("%s: Datapath %s psample action", dpif_name(backer->dpif), + supported ? "supports" : "does not support"); + return supported; +} + #define CHECK_FEATURE__(NAME, SUPPORT, FIELD, VALUE, ETHTYPE) \ static bool \ check_##NAME(struct dpif_backer *backer) \ @@ -1698,6 +1742,7 @@ check_support(struct dpif_backer *backer) dpif_supports_lb_output_action(backer->dpif); backer->rt_support.ct_zero_snat = dpif_supports_ct_zero_snat(backer); backer->rt_support.add_mpls = check_add_mpls(backer); + backer->rt_support.psample = check_psample(backer); /* Flow fields. */ backer->rt_support.odp.ct_state = check_ct_state(backer); @@ -5821,6 +5866,7 @@ get_datapath_cap(const char *datapath_type, struct smap *cap) smap_add(cap, "lb_output_action", s->lb_output_action ? "true" : "false"); smap_add(cap, "ct_zero_snat", s->ct_zero_snat ? "true" : "false"); smap_add(cap, "add_mpls", s->add_mpls ? "true" : "false"); + smap_add(cap, "psample", s->psample ? "true" : "false"); /* The ct_tuple_flush is implemented on dpif level, so it is supported * for all backers. */ diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h index d33f73df8..ea18ccedf 100644 --- a/ofproto/ofproto-dpif.h +++ b/ofproto/ofproto-dpif.h @@ -213,7 +213,10 @@ struct group_dpif *group_dpif_lookup(struct ofproto_dpif *, DPIF_SUPPORT_FIELD(bool, ct_zero_snat, "Conntrack all-zero IP SNAT") \ \ /* True if the datapath supports add_mpls action. */ \ - DPIF_SUPPORT_FIELD(bool, add_mpls, "MPLS Label add") + DPIF_SUPPORT_FIELD(bool, add_mpls, "MPLS Label add") \ + \ + /* True if the datapath supports psample action. */ \ + DPIF_SUPPORT_FIELD(bool, psample, "psample") /* Stores the various features which the corresponding backer supports. */ @@ -411,5 +414,6 @@ bool ofproto_dpif_ct_zone_timeout_policy_get_name( uint8_t nw_proto, char **tp_name, bool *unwildcard); bool ovs_explicit_drop_action_supported(struct ofproto_dpif *); +bool ovs_psample_supported(struct ofproto_dpif *); #endif /* ofproto-dpif.h */ diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index e3afb78a4..c6eb598d3 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -6511,6 +6511,11 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \ called <code>NXT_CT_FLUSH</code>. The <code>NXT_CT_FLUSH</code> extensions allows to flush CT entries based on specified parameters. </column> + <column name="capabilities" key="psample" + type='{"type": "boolean"}'> + True if the datapath supports OVS_ACTION_ATTR_PSAMPLE. If false, + local sampling will not be supported. + </column> </group> <column name="ct_zone_default_limit">
Only kernel datapath supports this action so add a function in dpif.c that checks for that. Signed-off-by: Adrian Moreno <amorenoz@redhat.com> --- lib/dpif.c | 7 +++++++ lib/dpif.h | 1 + ofproto/ofproto-dpif.c | 46 ++++++++++++++++++++++++++++++++++++++++++ ofproto/ofproto-dpif.h | 6 +++++- vswitchd/vswitch.xml | 5 +++++ 5 files changed, 64 insertions(+), 1 deletion(-)