| Message ID | 159904156402.95561.16528503305820427349.stgit@ebuild |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | dpctl: cache visibility/configuration enhancements | expand |
On Wed, Sep 02, 2020 at 12:12:44PM +0200, Eelco Chaudron wrote: > This patch adds a general way of viewing/configuring datapath > cache sizes. With an implementation for the netlink interface. I see you added support to multiple cache levels. Does that mean we can use to set sizes of EMC and SMC? > The ovs-dpctl/ovs-appctl show commands will display the > current cache sizes configured: > > ovs-dpctl show > system@ovs-system: > lookups: hit:25 missed:63 lost:0 > flows: 0 > masks: hit:282 total:0 hit/pkt:3.20 > cache: hit:4 hit rate:4.5455% > caches: > masks cache: size: 256 > port 0: ovs-system (internal) > port 1: br-int (internal) > port 2: genev_sys_6081 (geneve: packet_type=ptap) > port 3: br-ex (internal) > port 4: eth2 > port 5: sw0p1 (internal) > port 6: sw0p3 (internal) > > A specific cache can be configured as follows: > > ovs-appctl dpctl/cache-set-size DP CACHE SIZE > ovs-dpctl cache-set-size DP CACHE SIZE > > For example to disable the cache do: > > $ ovs-dpctl cache-set-size system@ovs-system "masks cache" 0 Should we use cache names with spaces? > Setting cache size successful, new size 0. > > Signed-off-by: Eelco Chaudron <echaudro@redhat.com> > --- > datapath/linux/compat/include/linux/openvswitch.h | 1 > lib/dpctl.c | 120 +++++++++++++++++++++ > lib/dpif-netdev.c | 4 + > lib/dpif-netlink.c | 113 ++++++++++++++++++++ > lib/dpif-provider.h | 20 ++++ > lib/dpif.c | 32 ++++++ > lib/dpif.h | 7 + > utilities/ovs-dpctl.c | 4 + > 8 files changed, 301 insertions(+) > > diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h > index fb73bfa90..4d4ead8af 100644 > --- a/datapath/linux/compat/include/linux/openvswitch.h > +++ b/datapath/linux/compat/include/linux/openvswitch.h > @@ -105,6 +105,7 @@ enum ovs_datapath_attr { > OVS_DP_ATTR_MEGAFLOW_STATS, /* struct ovs_dp_megaflow_stats */ > OVS_DP_ATTR_USER_FEATURES, /* OVS_DP_F_* */ > OVS_DP_ATTR_PAD, > + OVS_DP_ATTR_MASKS_CACHE_SIZE, > __OVS_DP_ATTR_MAX > }; > > diff --git a/lib/dpctl.c b/lib/dpctl.c > index dac350fb5..c8e8b3cdb 100644 > --- a/lib/dpctl.c > +++ b/lib/dpctl.c > @@ -585,6 +585,36 @@ compare_port_nos(const void *a_, const void *b_) > return a < b ? -1 : a > b; > } > > +static void > +show_dpif_cache__(struct dpif *dpif, struct dpctl_params *dpctl_p) > +{ > + uint32_t nr_caches; > + int error = dpif_cache_get_supported_levels(dpif, &nr_caches); > + > + if (error || nr_caches == 0) { > + return; > + } > + > + dpctl_print(dpctl_p, " caches:\n"); > + for (int i = 0; i < nr_caches; i++) { > + const char *name; > + uint32_t size; > + > + if (dpif_cache_get_name(dpif, i, &name) || > + dpif_cache_get_size(dpif, i, &size)) { > + continue; > + } > + dpctl_print(dpctl_p, " %s: size: %u\n", name, size); > + } > +} > + > +static void > +show_dpif_cache(struct dpif *dpif, struct dpctl_params *dpctl_p) > +{ > + dpctl_print(dpctl_p, "%s:\n", dpif_name(dpif)); > + show_dpif_cache__(dpif, dpctl_p); > +} > + > static void > show_dpif(struct dpif *dpif, struct dpctl_params *dpctl_p) > { > @@ -617,6 +647,8 @@ show_dpif(struct dpif *dpif, struct dpctl_params *dpctl_p) > } > } > > + show_dpif_cache__(dpif, dpctl_p); > + > odp_port_t *port_nos = NULL; > size_t allocated_port_nos = 0, n_port_nos = 0; > DPIF_PORT_FOR_EACH (&dpif_port, &dump, dpif) { > @@ -2224,6 +2256,92 @@ dpctl_ct_ipf_get_status(int argc, const char *argv[], > return error; > } > > +static int > +dpctl_cache_get_size(int argc, const char *argv[], > + struct dpctl_params *dpctl_p) > +{ > + int error; > + > + if (argc > 1) { > + struct dpif *dpif; > + > + error = parsed_dpif_open(argv[1], false, &dpif); > + if (!error) { > + show_dpif_cache(dpif, dpctl_p); > + dpif_close(dpif); > + } else { > + dpctl_error(dpctl_p, error, "opening datapath %s failed", argv[1]); > + } > + } else { > + error = dps_for_each(dpctl_p, show_dpif_cache); > + } > + > + return error; > +} > + > +static int > +dpctl_cache_set_size(int argc, const char *argv[], > + struct dpctl_params *dpctl_p) > +{ > + int i, error = EINVAL; > + uint32_t nr_caches, size; > + struct dpif *dpif; > + > + if (argc != 4) { > + dpctl_error(dpctl_p, error, "Invalid number of arguments passes."); s/passes// ? > + return error; > + } > + > + if (!ovs_scan(argv[3], "%"SCNu32, &size)) { > + dpctl_error(dpctl_p, error, "size seems malformed."); s/seems/is/ ? > + return error; > + } > + > + error = parsed_dpif_open(argv[1], false, &dpif); > + if (error) { > + dpctl_error(dpctl_p, error, "Opening datapath %s failed.", > + argv[1]); > + return error; > + } > + > + error = dpif_cache_get_supported_levels(dpif, &nr_caches); > + if (error || nr_caches == 0) { > + dpctl_error(dpctl_p, error, "Setting caches not supported."); > + goto exit; > + } > + > + for (i = 0; i < nr_caches; i++) { > + const char *name; > + > + if (dpif_cache_get_name(dpif, i, &name)) { > + continue; > + } > + if (!strcmp(argv[2], name)) { Shouldn't it use strncmp? > + break; > + } > + } > + > + if (i == nr_caches) { > + dpctl_error(dpctl_p, error, "Caches name \"%s\" no found on dpif.", > + argv[2]); s/Caches/Cache/ ? > + error = EINVAL; > + goto exit; > + } > + > + error = dpif_cache_set_size(dpif, i, size); > + if (!error) { > + dpif_cache_get_size(dpif, i, &size); > + dpctl_print(dpctl_p, "Setting cache size successful, new size %u.\n", > + size); > + } else { > + dpctl_error(dpctl_p, error, "Setting cache size failed!"); > + } > + > +exit: > + dpif_close(dpif); > + return error; > +} > + > /* Undocumented commands for unit testing. */ > > static int > @@ -2522,6 +2640,8 @@ static const struct dpctl_command all_commands[] = { > { "dump-conntrack", "[dp] [zone=N]", 0, 2, dpctl_dump_conntrack, DP_RO }, > { "flush-conntrack", "[dp] [zone=N] [ct-tuple]", 0, 3, > dpctl_flush_conntrack, DP_RW }, > + { "cache-get-size", "[dp]", 0, 1, dpctl_cache_get_size, DP_RO }, > + { "cache-set-size", "dp cache <size>", 3, 3, dpctl_cache_set_size, DP_RW }, Do you mind to update lib/dpctl.man as well ? > { "ct-stats-show", "[dp] [zone=N]", > 0, 3, dpctl_ct_stats_show, DP_RO }, > { "ct-bkts", "[dp] [gt=N]", 0, 2, dpctl_ct_bkts, DP_RO }, > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index 4e3bd7519..8a14c71aa 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -8465,6 +8465,10 @@ const struct dpif_class dpif_netdev_class = { > dpif_netdev_bond_add, > dpif_netdev_bond_del, > dpif_netdev_bond_stats_get, > + NULL, /* cache_get_supported_levels */ > + NULL, /* cache_get_name */ > + NULL, /* cache_get_size */ > + NULL, /* cache_set_size */ > }; > > static void > diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c > index 07f15ac65..dcc1d5e57 100644 > --- a/lib/dpif-netlink.c > +++ b/lib/dpif-netlink.c > @@ -95,6 +95,7 @@ struct dpif_netlink_dp { > const char *name; /* OVS_DP_ATTR_NAME. */ > const uint32_t *upcall_pid; /* OVS_DP_ATTR_UPCALL_PID. */ > uint32_t user_features; /* OVS_DP_ATTR_USER_FEATURES */ > + uint32_t cache_size; /* OVS_DP_ATTR_MASKS_CACHE_SIZE */ > const struct ovs_dp_stats *stats; /* OVS_DP_ATTR_STATS. */ > const struct ovs_dp_megaflow_stats *megaflow_stats; > /* OVS_DP_ATTR_MEGAFLOW_STATS.*/ > @@ -3983,6 +3984,100 @@ probe_broken_meters(struct dpif *dpif) > } > return broken_meters; > } > + > + > +static int > +dpif_netlink_cache_get_supported_levels(struct dpif *dpif_, uint32_t *levels) > +{ > + int error; > + struct ofpbuf *buf; > + struct dpif_netlink_dp dp; > + > + /* If available, in the kernel we support one level of cache. > + * Unfortunately, there is no way to detect if the older kernel module has > + * the cache feature. For now, we only report the cache information if the > + * kernel module reports the OVS_DP_ATTR_MASKS_CACHE_SIZE attribute. */ > + > + *levels = 0; > + error = dpif_netlink_dp_get(dpif_, &dp, &buf); > + if (!error) { > + > + if (dp.cache_size != UINT32_MAX) { > + *levels = 1; > + } > + ofpbuf_delete(buf); > + } > + > + return error; > +} > + > +static int > +dpif_netlink_cache_get_name(struct dpif *dpif_ OVS_UNUSED, uint32_t level, > + const char **name) > +{ > + if (level != 0) { > + return EINVAL; > + } > + > + *name = "masks cache"; As I mentioned before, maybe we should use "masks-cache" ? Just a suggestion. > + return 0; > +} > + > +static int > +dpif_netlink_cache_get_size(struct dpif *dpif_, uint32_t level, uint32_t *size) > +{ > + int error; > + struct ofpbuf *buf; > + struct dpif_netlink_dp dp; > + > + if (level != 0) { > + return EINVAL; > + } > + > + error = dpif_netlink_dp_get(dpif_, &dp, &buf); > + if (!error) { > + > + ofpbuf_delete(buf); > + > + if (dp.cache_size == UINT32_MAX) { > + return EOPNOTSUPP; > + } > + *size = dp.cache_size; > + } > + return error; > +} > + > +static int > +dpif_netlink_cache_set_size(struct dpif *dpif_, uint32_t level, uint32_t size) > +{ > + int error; > + struct ofpbuf *bufp; > + struct dpif_netlink_dp request, reply; > + struct dpif_netlink *dpif = dpif_netlink_cast(dpif_); > + > + size = ROUND_UP_POW2(size); > + > + if (level != 0) { > + return EINVAL; > + } > + > + dpif_netlink_dp_init(&request); > + request.cmd = OVS_DP_CMD_SET; > + request.name = dpif_->base_name; > + request.dp_ifindex = dpif->dp_ifindex; > + request.cache_size = size; > + > + error = dpif_netlink_dp_transact(&request, &reply, &bufp); > + if (!error) { > + ofpbuf_delete(bufp); > + if (reply.cache_size != size) { > + return EINVAL; > + } > + } > + > + return error; > +} > + > > const struct dpif_class dpif_netlink_class = { > "system", > @@ -4060,6 +4155,10 @@ const struct dpif_class dpif_netlink_class = { > NULL, /* bond_add */ > NULL, /* bond_del */ > NULL, /* bond_stats_get */ > + dpif_netlink_cache_get_supported_levels, > + dpif_netlink_cache_get_name, > + dpif_netlink_cache_get_size, > + dpif_netlink_cache_set_size, > }; > > static int > @@ -4320,6 +4419,9 @@ dpif_netlink_dp_from_ofpbuf(struct dpif_netlink_dp *dp, const struct ofpbuf *buf > [OVS_DP_ATTR_USER_FEATURES] = { > .type = NL_A_U32, > .optional = true }, > + [OVS_DP_ATTR_MASKS_CACHE_SIZE] = { > + .type = NL_A_U32, > + .optional = true }, > }; > > dpif_netlink_dp_init(dp); > @@ -4352,6 +4454,12 @@ dpif_netlink_dp_from_ofpbuf(struct dpif_netlink_dp *dp, const struct ofpbuf *buf > dp->user_features = nl_attr_get_u32(a[OVS_DP_ATTR_USER_FEATURES]); > } > > + if (a[OVS_DP_ATTR_MASKS_CACHE_SIZE]) { > + dp->cache_size = nl_attr_get_u32(a[OVS_DP_ATTR_MASKS_CACHE_SIZE]); > + } else { > + dp->cache_size = UINT32_MAX; > + } > + > return 0; > } > > @@ -4380,6 +4488,10 @@ dpif_netlink_dp_to_ofpbuf(const struct dpif_netlink_dp *dp, struct ofpbuf *buf) > nl_msg_put_u32(buf, OVS_DP_ATTR_USER_FEATURES, dp->user_features); > } > > + if (dp->cache_size != UINT32_MAX) { > + nl_msg_put_u32(buf, OVS_DP_ATTR_MASKS_CACHE_SIZE, dp->cache_size); > + } > + > /* Skip OVS_DP_ATTR_STATS since we never have a reason to serialize it. */ > } > > @@ -4388,6 +4500,7 @@ static void > dpif_netlink_dp_init(struct dpif_netlink_dp *dp) > { > memset(dp, 0, sizeof *dp); > + dp->cache_size = UINT32_MAX; > } > > static void > diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h > index 0e024c1c9..61bd90f57 100644 > --- a/lib/dpif-provider.h > +++ b/lib/dpif-provider.h > @@ -628,6 +628,26 @@ struct dpif_class { > * sufficient to store BOND_BUCKETS number of elements. */ > int (*bond_stats_get)(struct dpif *dpif, uint32_t bond_id, > uint64_t *n_bytes); > + > + /* Cache configuration > + * > + * Multiple levels of cache can exist in a given datapath implementation. > + * An API has been provided to get the number of supported caches, which > + * can then be used to get/set specific configuration. Cache level is 0 > + * indexed, i.e. if 1 level is supported, the level value to use is 0. > + * > + * Get the number of cache levels supported. */ > + int (*cache_get_supported_levels)(struct dpif *dpif, uint32_t *levels); > + > + /* Get the cache name for the given level. */ > + int (*cache_get_name)(struct dpif *dpif, uint32_t level, > + const char **name); > + > + /* Get currently configured cache size. */ > + int (*cache_get_size)(struct dpif *dpif, uint32_t level, uint32_t *size); > + > + /* Set cache size. */ > + int (*cache_set_size)(struct dpif *dpif, uint32_t level, uint32_t size); > }; > > extern const struct dpif_class dpif_netlink_class; > diff --git a/lib/dpif.c b/lib/dpif.c > index 7cac3a629..4db897c7d 100644 > --- a/lib/dpif.c > +++ b/lib/dpif.c > @@ -2018,3 +2018,35 @@ dpif_bond_stats_get(struct dpif *dpif, uint32_t bond_id, > ? dpif->dpif_class->bond_stats_get(dpif, bond_id, n_bytes) > : EOPNOTSUPP; > } > + > +int > +dpif_cache_get_supported_levels(struct dpif *dpif, uint32_t *levels) > +{ > + return dpif->dpif_class->cache_get_supported_levels > + ? dpif->dpif_class->cache_get_supported_levels(dpif, levels) > + : EOPNOTSUPP; > +} > + > +int > +dpif_cache_get_name(struct dpif *dpif, uint32_t level, const char **name) > +{ > + return dpif->dpif_class->cache_get_name > + ? dpif->dpif_class->cache_get_name(dpif, level, name) > + : EOPNOTSUPP; > +} > + > +int > +dpif_cache_get_size(struct dpif *dpif, uint32_t level, uint32_t *size) > +{ > + return dpif->dpif_class->cache_get_size > + ? dpif->dpif_class->cache_get_size(dpif, level, size) > + : EOPNOTSUPP; > +} > + > +int > +dpif_cache_set_size(struct dpif *dpif, uint32_t level, uint32_t size) > +{ > + return dpif->dpif_class->cache_set_size > + ? dpif->dpif_class->cache_set_size(dpif, level, size) > + : EOPNOTSUPP; > +} > diff --git a/lib/dpif.h b/lib/dpif.h > index 43c1ab998..35f48bec6 100644 > --- a/lib/dpif.h > +++ b/lib/dpif.h > @@ -905,6 +905,13 @@ int dpif_bond_del(struct dpif *, uint32_t bond_id); > int dpif_bond_stats_get(struct dpif *, uint32_t bond_id, uint64_t *n_bytes); > bool dpif_supports_lb_output_action(const struct dpif *); > > + > +/* Cache */ > +int dpif_cache_get_supported_levels(struct dpif *dpif, uint32_t *levels); > +int dpif_cache_get_name(struct dpif *dpif, uint32_t level, const char **name); > +int dpif_cache_get_size(struct dpif *dpif, uint32_t level, uint32_t *size); > +int dpif_cache_set_size(struct dpif *dpif, uint32_t level, uint32_t size); > + > > /* Miscellaneous. */ > > diff --git a/utilities/ovs-dpctl.c b/utilities/ovs-dpctl.c > index 7d99607f4..66055cfe2 100644 > --- a/utilities/ovs-dpctl.c > +++ b/utilities/ovs-dpctl.c > @@ -195,6 +195,10 @@ usage(void *userdata OVS_UNUSED) > " get-flow [DP] ufid:UFID fetch flow corresponding to UFID\n" > " del-flow [DP] FLOW delete FLOW from DP\n" > " del-flows [DP] delete all flows from DP\n" > + " cache-get-size [DP] " \ > + "Show the current size for all caches\n" > + " cache-set-size DP CACHE SIZE " \ > + "Set cache size for a specific cache\n" > " dump-conntrack [DP] [zone=ZONE] " \ > "display conntrack entries for ZONE\n" > " flush-conntrack [DP] [zone=ZONE] [ct-tuple]" \ Do you think it is possible to include a check-kernel test unit? I.e. if the running version is above the one with the introduced cache size feature, try to set and get it back otherwise skip? Thanks, fbl
On 1 Mar 2021, at 22:04, Flavio Leitner wrote: > On Wed, Sep 02, 2020 at 12:12:44PM +0200, Eelco Chaudron wrote: >> This patch adds a general way of viewing/configuring datapath >> cache sizes. With an implementation for the netlink interface. > > I see you added support to multiple cache levels. Does that > mean we can use to set sizes of EMC and SMC? Yes, the idea was that the same API can be used for the userspace EMC/SMC. >> The ovs-dpctl/ovs-appctl show commands will display the >> current cache sizes configured: >> >> ovs-dpctl show >> system@ovs-system: >> lookups: hit:25 missed:63 lost:0 >> flows: 0 >> masks: hit:282 total:0 hit/pkt:3.20 >> cache: hit:4 hit rate:4.5455% >> caches: >> masks cache: size: 256 >> port 0: ovs-system (internal) >> port 1: br-int (internal) >> port 2: genev_sys_6081 (geneve: packet_type=ptap) >> port 3: br-ex (internal) >> port 4: eth2 >> port 5: sw0p1 (internal) >> port 6: sw0p3 (internal) >> >> A specific cache can be configured as follows: >> >> ovs-appctl dpctl/cache-set-size DP CACHE SIZE >> ovs-dpctl cache-set-size DP CACHE SIZE >> >> For example to disable the cache do: >> >> $ ovs-dpctl cache-set-size system@ovs-system "masks cache" 0 > > Should we use cache names with spaces? I thought that with a space, it looked more aesthetic, however, it's more cumbersome to use, i.e. it needs quotes. So I changed it to use a hyphen in the v2, which I will send out soon. >> Setting cache size successful, new size 0. >> >> Signed-off-by: Eelco Chaudron <echaudro@redhat.com> >> --- >> datapath/linux/compat/include/linux/openvswitch.h | 1 >> lib/dpctl.c | 120 >> +++++++++++++++++++++ >> lib/dpif-netdev.c | 4 + >> lib/dpif-netlink.c | 113 >> ++++++++++++++++++++ >> lib/dpif-provider.h | 20 ++++ >> lib/dpif.c | 32 ++++++ >> lib/dpif.h | 7 + >> utilities/ovs-dpctl.c | 4 + >> 8 files changed, 301 insertions(+) >> >> diff --git a/datapath/linux/compat/include/linux/openvswitch.h >> b/datapath/linux/compat/include/linux/openvswitch.h >> index fb73bfa90..4d4ead8af 100644 >> --- a/datapath/linux/compat/include/linux/openvswitch.h >> +++ b/datapath/linux/compat/include/linux/openvswitch.h >> @@ -105,6 +105,7 @@ enum ovs_datapath_attr { >> OVS_DP_ATTR_MEGAFLOW_STATS, /* struct ovs_dp_megaflow_stats */ >> OVS_DP_ATTR_USER_FEATURES, /* OVS_DP_F_* */ >> OVS_DP_ATTR_PAD, >> + OVS_DP_ATTR_MASKS_CACHE_SIZE, >> __OVS_DP_ATTR_MAX >> }; >> >> diff --git a/lib/dpctl.c b/lib/dpctl.c >> index dac350fb5..c8e8b3cdb 100644 >> --- a/lib/dpctl.c >> +++ b/lib/dpctl.c >> @@ -585,6 +585,36 @@ compare_port_nos(const void *a_, const void *b_) >> return a < b ? -1 : a > b; >> } >> >> +static void >> +show_dpif_cache__(struct dpif *dpif, struct dpctl_params *dpctl_p) >> +{ >> + uint32_t nr_caches; >> + int error = dpif_cache_get_supported_levels(dpif, &nr_caches); >> + >> + if (error || nr_caches == 0) { >> + return; >> + } >> + >> + dpctl_print(dpctl_p, " caches:\n"); >> + for (int i = 0; i < nr_caches; i++) { >> + const char *name; >> + uint32_t size; >> + >> + if (dpif_cache_get_name(dpif, i, &name) || >> + dpif_cache_get_size(dpif, i, &size)) { >> + continue; >> + } >> + dpctl_print(dpctl_p, " %s: size: %u\n", name, size); >> + } >> +} >> + >> +static void >> +show_dpif_cache(struct dpif *dpif, struct dpctl_params *dpctl_p) >> +{ >> + dpctl_print(dpctl_p, "%s:\n", dpif_name(dpif)); >> + show_dpif_cache__(dpif, dpctl_p); >> +} >> + >> static void >> show_dpif(struct dpif *dpif, struct dpctl_params *dpctl_p) >> { >> @@ -617,6 +647,8 @@ show_dpif(struct dpif *dpif, struct dpctl_params >> *dpctl_p) >> } >> } >> >> + show_dpif_cache__(dpif, dpctl_p); >> + >> odp_port_t *port_nos = NULL; >> size_t allocated_port_nos = 0, n_port_nos = 0; >> DPIF_PORT_FOR_EACH (&dpif_port, &dump, dpif) { >> @@ -2224,6 +2256,92 @@ dpctl_ct_ipf_get_status(int argc, const char >> *argv[], >> return error; >> } >> >> +static int >> +dpctl_cache_get_size(int argc, const char *argv[], >> + struct dpctl_params *dpctl_p) >> +{ >> + int error; >> + >> + if (argc > 1) { >> + struct dpif *dpif; >> + >> + error = parsed_dpif_open(argv[1], false, &dpif); >> + if (!error) { >> + show_dpif_cache(dpif, dpctl_p); >> + dpif_close(dpif); >> + } else { >> + dpctl_error(dpctl_p, error, "opening datapath %s >> failed", argv[1]); >> + } >> + } else { >> + error = dps_for_each(dpctl_p, show_dpif_cache); >> + } >> + >> + return error; >> +} >> + >> +static int >> +dpctl_cache_set_size(int argc, const char *argv[], >> + struct dpctl_params *dpctl_p) >> +{ >> + int i, error = EINVAL; >> + uint32_t nr_caches, size; >> + struct dpif *dpif; >> + >> + if (argc != 4) { >> + dpctl_error(dpctl_p, error, "Invalid number of arguments >> passes."); > > s/passes// ? Will fix in V2 >> + return error; >> + } >> + >> + if (!ovs_scan(argv[3], "%"SCNu32, &size)) { >> + dpctl_error(dpctl_p, error, "size seems malformed."); > > s/seems/is/ ? Will fix in V2 >> + return error; >> + } >> + >> + error = parsed_dpif_open(argv[1], false, &dpif); >> + if (error) { >> + dpctl_error(dpctl_p, error, "Opening datapath %s >> failed.", >> + argv[1]); >> + return error; >> + } >> + >> + error = dpif_cache_get_supported_levels(dpif, &nr_caches); >> + if (error || nr_caches == 0) { >> + dpctl_error(dpctl_p, error, "Setting caches not >> supported."); >> + goto exit; >> + } >> + >> + for (i = 0; i < nr_caches; i++) { >> + const char *name; >> + >> + if (dpif_cache_get_name(dpif, i, &name)) { >> + continue; >> + } >> + if (!strcmp(argv[2], name)) { > > Shouldn't it use strncmp? It needs a full match, so no need for strncmp. >> + break; >> + } >> + } >> + >> + if (i == nr_caches) { >> + dpctl_error(dpctl_p, error, "Caches name \"%s\" no found on >> dpif.", >> + argv[2]); > > s/Caches/Cache/ ? Will fix in V2 >> + error = EINVAL; >> + goto exit; >> + } >> + >> + error = dpif_cache_set_size(dpif, i, size); >> + if (!error) { >> + dpif_cache_get_size(dpif, i, &size); >> + dpctl_print(dpctl_p, "Setting cache size successful, new >> size %u.\n", >> + size); >> + } else { >> + dpctl_error(dpctl_p, error, "Setting cache size failed!"); >> + } >> + >> +exit: >> + dpif_close(dpif); >> + return error; >> +} >> + >> /* Undocumented commands for unit testing. */ >> >> static int >> @@ -2522,6 +2640,8 @@ static const struct dpctl_command >> all_commands[] = { >> { "dump-conntrack", "[dp] [zone=N]", 0, 2, dpctl_dump_conntrack, >> DP_RO }, >> { "flush-conntrack", "[dp] [zone=N] [ct-tuple]", 0, 3, >> dpctl_flush_conntrack, DP_RW }, >> + { "cache-get-size", "[dp]", 0, 1, dpctl_cache_get_size, DP_RO }, >> + { "cache-set-size", "dp cache <size>", 3, 3, >> dpctl_cache_set_size, DP_RW }, > > > Do you mind to update lib/dpctl.man as well ? Will add in V2 >> { "ct-stats-show", "[dp] [zone=N]", >> 0, 3, dpctl_ct_stats_show, DP_RO }, >> { "ct-bkts", "[dp] [gt=N]", 0, 2, dpctl_ct_bkts, DP_RO }, >> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c >> index 4e3bd7519..8a14c71aa 100644 >> --- a/lib/dpif-netdev.c >> +++ b/lib/dpif-netdev.c >> @@ -8465,6 +8465,10 @@ const struct dpif_class dpif_netdev_class = { >> dpif_netdev_bond_add, >> dpif_netdev_bond_del, >> dpif_netdev_bond_stats_get, >> + NULL, /* cache_get_supported_levels */ >> + NULL, /* cache_get_name */ >> + NULL, /* cache_get_size */ >> + NULL, /* cache_set_size */ >> }; >> >> static void >> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c >> index 07f15ac65..dcc1d5e57 100644 >> --- a/lib/dpif-netlink.c >> +++ b/lib/dpif-netlink.c >> @@ -95,6 +95,7 @@ struct dpif_netlink_dp { >> const char *name; /* OVS_DP_ATTR_NAME. */ >> const uint32_t *upcall_pid; /* OVS_DP_ATTR_UPCALL_PID. */ >> uint32_t user_features; /* OVS_DP_ATTR_USER_FEATURES >> */ >> + uint32_t cache_size; /* >> OVS_DP_ATTR_MASKS_CACHE_SIZE */ >> const struct ovs_dp_stats *stats; /* OVS_DP_ATTR_STATS. */ >> const struct ovs_dp_megaflow_stats *megaflow_stats; >> /* >> OVS_DP_ATTR_MEGAFLOW_STATS.*/ >> @@ -3983,6 +3984,100 @@ probe_broken_meters(struct dpif *dpif) >> } >> return broken_meters; >> } >> + >> + >> +static int >> +dpif_netlink_cache_get_supported_levels(struct dpif *dpif_, uint32_t >> *levels) >> +{ >> + int error; >> + struct ofpbuf *buf; >> + struct dpif_netlink_dp dp; >> + >> + /* If available, in the kernel we support one level of cache. >> + * Unfortunately, there is no way to detect if the older kernel >> module has >> + * the cache feature. For now, we only report the cache >> information if the >> + * kernel module reports the OVS_DP_ATTR_MASKS_CACHE_SIZE >> attribute. */ >> + >> + *levels = 0; >> + error = dpif_netlink_dp_get(dpif_, &dp, &buf); >> + if (!error) { >> + >> + if (dp.cache_size != UINT32_MAX) { >> + *levels = 1; >> + } >> + ofpbuf_delete(buf); >> + } >> + >> + return error; >> +} >> + >> +static int >> +dpif_netlink_cache_get_name(struct dpif *dpif_ OVS_UNUSED, uint32_t >> level, >> + const char **name) >> +{ >> + if (level != 0) { >> + return EINVAL; >> + } >> + >> + *name = "masks cache"; > > As I mentioned before, maybe we should use "masks-cache" ? > Just a suggestion. Done! >> + return 0; >> +} >> + >> +static int >> +dpif_netlink_cache_get_size(struct dpif *dpif_, uint32_t level, >> uint32_t *size) >> +{ >> + int error; >> + struct ofpbuf *buf; >> + struct dpif_netlink_dp dp; >> + >> + if (level != 0) { >> + return EINVAL; >> + } >> + >> + error = dpif_netlink_dp_get(dpif_, &dp, &buf); >> + if (!error) { >> + >> + ofpbuf_delete(buf); >> + >> + if (dp.cache_size == UINT32_MAX) { >> + return EOPNOTSUPP; >> + } >> + *size = dp.cache_size; >> + } >> + return error; >> +} >> + >> +static int >> +dpif_netlink_cache_set_size(struct dpif *dpif_, uint32_t level, >> uint32_t size) >> +{ >> + int error; >> + struct ofpbuf *bufp; >> + struct dpif_netlink_dp request, reply; >> + struct dpif_netlink *dpif = dpif_netlink_cast(dpif_); >> + >> + size = ROUND_UP_POW2(size); >> + >> + if (level != 0) { >> + return EINVAL; >> + } >> + >> + dpif_netlink_dp_init(&request); >> + request.cmd = OVS_DP_CMD_SET; >> + request.name = dpif_->base_name; >> + request.dp_ifindex = dpif->dp_ifindex; >> + request.cache_size = size; >> + >> + error = dpif_netlink_dp_transact(&request, &reply, &bufp); >> + if (!error) { >> + ofpbuf_delete(bufp); >> + if (reply.cache_size != size) { >> + return EINVAL; >> + } >> + } >> + >> + return error; >> +} >> + >> >> const struct dpif_class dpif_netlink_class = { >> "system", >> @@ -4060,6 +4155,10 @@ const struct dpif_class dpif_netlink_class = { >> NULL, /* bond_add */ >> NULL, /* bond_del */ >> NULL, /* bond_stats_get */ >> + dpif_netlink_cache_get_supported_levels, >> + dpif_netlink_cache_get_name, >> + dpif_netlink_cache_get_size, >> + dpif_netlink_cache_set_size, >> }; >> >> static int >> @@ -4320,6 +4419,9 @@ dpif_netlink_dp_from_ofpbuf(struct >> dpif_netlink_dp *dp, const struct ofpbuf *buf >> [OVS_DP_ATTR_USER_FEATURES] = { >> .type = NL_A_U32, >> .optional = true }, >> + [OVS_DP_ATTR_MASKS_CACHE_SIZE] = { >> + .type = NL_A_U32, >> + .optional = true }, >> }; >> >> dpif_netlink_dp_init(dp); >> @@ -4352,6 +4454,12 @@ dpif_netlink_dp_from_ofpbuf(struct >> dpif_netlink_dp *dp, const struct ofpbuf *buf >> dp->user_features = >> nl_attr_get_u32(a[OVS_DP_ATTR_USER_FEATURES]); >> } >> >> + if (a[OVS_DP_ATTR_MASKS_CACHE_SIZE]) { >> + dp->cache_size = >> nl_attr_get_u32(a[OVS_DP_ATTR_MASKS_CACHE_SIZE]); >> + } else { >> + dp->cache_size = UINT32_MAX; >> + } >> + >> return 0; >> } >> >> @@ -4380,6 +4488,10 @@ dpif_netlink_dp_to_ofpbuf(const struct >> dpif_netlink_dp *dp, struct ofpbuf *buf) >> nl_msg_put_u32(buf, OVS_DP_ATTR_USER_FEATURES, >> dp->user_features); >> } >> >> + if (dp->cache_size != UINT32_MAX) { >> + nl_msg_put_u32(buf, OVS_DP_ATTR_MASKS_CACHE_SIZE, >> dp->cache_size); >> + } >> + >> /* Skip OVS_DP_ATTR_STATS since we never have a reason to >> serialize it. */ >> } >> >> @@ -4388,6 +4500,7 @@ static void >> dpif_netlink_dp_init(struct dpif_netlink_dp *dp) >> { >> memset(dp, 0, sizeof *dp); >> + dp->cache_size = UINT32_MAX; >> } >> >> static void >> diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h >> index 0e024c1c9..61bd90f57 100644 >> --- a/lib/dpif-provider.h >> +++ b/lib/dpif-provider.h >> @@ -628,6 +628,26 @@ struct dpif_class { >> * sufficient to store BOND_BUCKETS number of elements. */ >> int (*bond_stats_get)(struct dpif *dpif, uint32_t bond_id, >> uint64_t *n_bytes); >> + >> + /* Cache configuration >> + * >> + * Multiple levels of cache can exist in a given datapath >> implementation. >> + * An API has been provided to get the number of supported >> caches, which >> + * can then be used to get/set specific configuration. Cache >> level is 0 >> + * indexed, i.e. if 1 level is supported, the level value to use >> is 0. >> + * >> + * Get the number of cache levels supported. */ >> + int (*cache_get_supported_levels)(struct dpif *dpif, uint32_t >> *levels); >> + >> + /* Get the cache name for the given level. */ >> + int (*cache_get_name)(struct dpif *dpif, uint32_t level, >> + const char **name); >> + >> + /* Get currently configured cache size. */ >> + int (*cache_get_size)(struct dpif *dpif, uint32_t level, >> uint32_t *size); >> + >> + /* Set cache size. */ >> + int (*cache_set_size)(struct dpif *dpif, uint32_t level, >> uint32_t size); >> }; >> >> extern const struct dpif_class dpif_netlink_class; >> diff --git a/lib/dpif.c b/lib/dpif.c >> index 7cac3a629..4db897c7d 100644 >> --- a/lib/dpif.c >> +++ b/lib/dpif.c >> @@ -2018,3 +2018,35 @@ dpif_bond_stats_get(struct dpif *dpif, >> uint32_t bond_id, >> ? dpif->dpif_class->bond_stats_get(dpif, bond_id, >> n_bytes) >> : EOPNOTSUPP; >> } >> + >> +int >> +dpif_cache_get_supported_levels(struct dpif *dpif, uint32_t *levels) >> +{ >> + return dpif->dpif_class->cache_get_supported_levels >> + ? dpif->dpif_class->cache_get_supported_levels(dpif, levels) >> + : EOPNOTSUPP; >> +} >> + >> +int >> +dpif_cache_get_name(struct dpif *dpif, uint32_t level, const char >> **name) >> +{ >> + return dpif->dpif_class->cache_get_name >> + ? dpif->dpif_class->cache_get_name(dpif, level, name) >> + : EOPNOTSUPP; >> +} >> + >> +int >> +dpif_cache_get_size(struct dpif *dpif, uint32_t level, uint32_t >> *size) >> +{ >> + return dpif->dpif_class->cache_get_size >> + ? dpif->dpif_class->cache_get_size(dpif, level, size) >> + : EOPNOTSUPP; >> +} >> + >> +int >> +dpif_cache_set_size(struct dpif *dpif, uint32_t level, uint32_t >> size) >> +{ >> + return dpif->dpif_class->cache_set_size >> + ? dpif->dpif_class->cache_set_size(dpif, level, size) >> + : EOPNOTSUPP; >> +} >> diff --git a/lib/dpif.h b/lib/dpif.h >> index 43c1ab998..35f48bec6 100644 >> --- a/lib/dpif.h >> +++ b/lib/dpif.h >> @@ -905,6 +905,13 @@ int dpif_bond_del(struct dpif *, uint32_t >> bond_id); >> int dpif_bond_stats_get(struct dpif *, uint32_t bond_id, uint64_t >> *n_bytes); >> bool dpif_supports_lb_output_action(const struct dpif *); >> >> + >> +/* Cache */ >> +int dpif_cache_get_supported_levels(struct dpif *dpif, uint32_t >> *levels); >> +int dpif_cache_get_name(struct dpif *dpif, uint32_t level, const >> char **name); >> +int dpif_cache_get_size(struct dpif *dpif, uint32_t level, uint32_t >> *size); >> +int dpif_cache_set_size(struct dpif *dpif, uint32_t level, uint32_t >> size); >> + >> >> /* Miscellaneous. */ >> >> diff --git a/utilities/ovs-dpctl.c b/utilities/ovs-dpctl.c >> index 7d99607f4..66055cfe2 100644 >> --- a/utilities/ovs-dpctl.c >> +++ b/utilities/ovs-dpctl.c >> @@ -195,6 +195,10 @@ usage(void *userdata OVS_UNUSED) >> " get-flow [DP] ufid:UFID fetch flow corresponding to >> UFID\n" >> " del-flow [DP] FLOW delete FLOW from DP\n" >> " del-flows [DP] delete all flows from DP\n" >> + " cache-get-size [DP] " \ >> + "Show the current size for all caches\n" >> + " cache-set-size DP CACHE SIZE " \ >> + "Set cache size for a specific cache\n" >> " dump-conntrack [DP] [zone=ZONE] " \ >> "display conntrack entries for ZONE\n" >> " flush-conntrack [DP] [zone=ZONE] [ct-tuple]" \ > > > > Do you think it is possible to include a check-kernel test unit? > I.e. if the running version is above the one with the introduced > cache size feature, try to set and get it back otherwise skip? Done in v2 > Thanks, > fbl
diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h index fb73bfa90..4d4ead8af 100644 --- a/datapath/linux/compat/include/linux/openvswitch.h +++ b/datapath/linux/compat/include/linux/openvswitch.h @@ -105,6 +105,7 @@ enum ovs_datapath_attr { OVS_DP_ATTR_MEGAFLOW_STATS, /* struct ovs_dp_megaflow_stats */ OVS_DP_ATTR_USER_FEATURES, /* OVS_DP_F_* */ OVS_DP_ATTR_PAD, + OVS_DP_ATTR_MASKS_CACHE_SIZE, __OVS_DP_ATTR_MAX }; diff --git a/lib/dpctl.c b/lib/dpctl.c index dac350fb5..c8e8b3cdb 100644 --- a/lib/dpctl.c +++ b/lib/dpctl.c @@ -585,6 +585,36 @@ compare_port_nos(const void *a_, const void *b_) return a < b ? -1 : a > b; } +static void +show_dpif_cache__(struct dpif *dpif, struct dpctl_params *dpctl_p) +{ + uint32_t nr_caches; + int error = dpif_cache_get_supported_levels(dpif, &nr_caches); + + if (error || nr_caches == 0) { + return; + } + + dpctl_print(dpctl_p, " caches:\n"); + for (int i = 0; i < nr_caches; i++) { + const char *name; + uint32_t size; + + if (dpif_cache_get_name(dpif, i, &name) || + dpif_cache_get_size(dpif, i, &size)) { + continue; + } + dpctl_print(dpctl_p, " %s: size: %u\n", name, size); + } +} + +static void +show_dpif_cache(struct dpif *dpif, struct dpctl_params *dpctl_p) +{ + dpctl_print(dpctl_p, "%s:\n", dpif_name(dpif)); + show_dpif_cache__(dpif, dpctl_p); +} + static void show_dpif(struct dpif *dpif, struct dpctl_params *dpctl_p) { @@ -617,6 +647,8 @@ show_dpif(struct dpif *dpif, struct dpctl_params *dpctl_p) } } + show_dpif_cache__(dpif, dpctl_p); + odp_port_t *port_nos = NULL; size_t allocated_port_nos = 0, n_port_nos = 0; DPIF_PORT_FOR_EACH (&dpif_port, &dump, dpif) { @@ -2224,6 +2256,92 @@ dpctl_ct_ipf_get_status(int argc, const char *argv[], return error; } +static int +dpctl_cache_get_size(int argc, const char *argv[], + struct dpctl_params *dpctl_p) +{ + int error; + + if (argc > 1) { + struct dpif *dpif; + + error = parsed_dpif_open(argv[1], false, &dpif); + if (!error) { + show_dpif_cache(dpif, dpctl_p); + dpif_close(dpif); + } else { + dpctl_error(dpctl_p, error, "opening datapath %s failed", argv[1]); + } + } else { + error = dps_for_each(dpctl_p, show_dpif_cache); + } + + return error; +} + +static int +dpctl_cache_set_size(int argc, const char *argv[], + struct dpctl_params *dpctl_p) +{ + int i, error = EINVAL; + uint32_t nr_caches, size; + struct dpif *dpif; + + if (argc != 4) { + dpctl_error(dpctl_p, error, "Invalid number of arguments passes."); + return error; + } + + if (!ovs_scan(argv[3], "%"SCNu32, &size)) { + dpctl_error(dpctl_p, error, "size seems malformed."); + return error; + } + + error = parsed_dpif_open(argv[1], false, &dpif); + if (error) { + dpctl_error(dpctl_p, error, "Opening datapath %s failed.", + argv[1]); + return error; + } + + error = dpif_cache_get_supported_levels(dpif, &nr_caches); + if (error || nr_caches == 0) { + dpctl_error(dpctl_p, error, "Setting caches not supported."); + goto exit; + } + + for (i = 0; i < nr_caches; i++) { + const char *name; + + if (dpif_cache_get_name(dpif, i, &name)) { + continue; + } + if (!strcmp(argv[2], name)) { + break; + } + } + + if (i == nr_caches) { + dpctl_error(dpctl_p, error, "Caches name \"%s\" no found on dpif.", + argv[2]); + error = EINVAL; + goto exit; + } + + error = dpif_cache_set_size(dpif, i, size); + if (!error) { + dpif_cache_get_size(dpif, i, &size); + dpctl_print(dpctl_p, "Setting cache size successful, new size %u.\n", + size); + } else { + dpctl_error(dpctl_p, error, "Setting cache size failed!"); + } + +exit: + dpif_close(dpif); + return error; +} + /* Undocumented commands for unit testing. */ static int @@ -2522,6 +2640,8 @@ static const struct dpctl_command all_commands[] = { { "dump-conntrack", "[dp] [zone=N]", 0, 2, dpctl_dump_conntrack, DP_RO }, { "flush-conntrack", "[dp] [zone=N] [ct-tuple]", 0, 3, dpctl_flush_conntrack, DP_RW }, + { "cache-get-size", "[dp]", 0, 1, dpctl_cache_get_size, DP_RO }, + { "cache-set-size", "dp cache <size>", 3, 3, dpctl_cache_set_size, DP_RW }, { "ct-stats-show", "[dp] [zone=N]", 0, 3, dpctl_ct_stats_show, DP_RO }, { "ct-bkts", "[dp] [gt=N]", 0, 2, dpctl_ct_bkts, DP_RO }, diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 4e3bd7519..8a14c71aa 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -8465,6 +8465,10 @@ const struct dpif_class dpif_netdev_class = { dpif_netdev_bond_add, dpif_netdev_bond_del, dpif_netdev_bond_stats_get, + NULL, /* cache_get_supported_levels */ + NULL, /* cache_get_name */ + NULL, /* cache_get_size */ + NULL, /* cache_set_size */ }; static void diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c index 07f15ac65..dcc1d5e57 100644 --- a/lib/dpif-netlink.c +++ b/lib/dpif-netlink.c @@ -95,6 +95,7 @@ struct dpif_netlink_dp { const char *name; /* OVS_DP_ATTR_NAME. */ const uint32_t *upcall_pid; /* OVS_DP_ATTR_UPCALL_PID. */ uint32_t user_features; /* OVS_DP_ATTR_USER_FEATURES */ + uint32_t cache_size; /* OVS_DP_ATTR_MASKS_CACHE_SIZE */ const struct ovs_dp_stats *stats; /* OVS_DP_ATTR_STATS. */ const struct ovs_dp_megaflow_stats *megaflow_stats; /* OVS_DP_ATTR_MEGAFLOW_STATS.*/ @@ -3983,6 +3984,100 @@ probe_broken_meters(struct dpif *dpif) } return broken_meters; } + + +static int +dpif_netlink_cache_get_supported_levels(struct dpif *dpif_, uint32_t *levels) +{ + int error; + struct ofpbuf *buf; + struct dpif_netlink_dp dp; + + /* If available, in the kernel we support one level of cache. + * Unfortunately, there is no way to detect if the older kernel module has + * the cache feature. For now, we only report the cache information if the + * kernel module reports the OVS_DP_ATTR_MASKS_CACHE_SIZE attribute. */ + + *levels = 0; + error = dpif_netlink_dp_get(dpif_, &dp, &buf); + if (!error) { + + if (dp.cache_size != UINT32_MAX) { + *levels = 1; + } + ofpbuf_delete(buf); + } + + return error; +} + +static int +dpif_netlink_cache_get_name(struct dpif *dpif_ OVS_UNUSED, uint32_t level, + const char **name) +{ + if (level != 0) { + return EINVAL; + } + + *name = "masks cache"; + return 0; +} + +static int +dpif_netlink_cache_get_size(struct dpif *dpif_, uint32_t level, uint32_t *size) +{ + int error; + struct ofpbuf *buf; + struct dpif_netlink_dp dp; + + if (level != 0) { + return EINVAL; + } + + error = dpif_netlink_dp_get(dpif_, &dp, &buf); + if (!error) { + + ofpbuf_delete(buf); + + if (dp.cache_size == UINT32_MAX) { + return EOPNOTSUPP; + } + *size = dp.cache_size; + } + return error; +} + +static int +dpif_netlink_cache_set_size(struct dpif *dpif_, uint32_t level, uint32_t size) +{ + int error; + struct ofpbuf *bufp; + struct dpif_netlink_dp request, reply; + struct dpif_netlink *dpif = dpif_netlink_cast(dpif_); + + size = ROUND_UP_POW2(size); + + if (level != 0) { + return EINVAL; + } + + dpif_netlink_dp_init(&request); + request.cmd = OVS_DP_CMD_SET; + request.name = dpif_->base_name; + request.dp_ifindex = dpif->dp_ifindex; + request.cache_size = size; + + error = dpif_netlink_dp_transact(&request, &reply, &bufp); + if (!error) { + ofpbuf_delete(bufp); + if (reply.cache_size != size) { + return EINVAL; + } + } + + return error; +} + const struct dpif_class dpif_netlink_class = { "system", @@ -4060,6 +4155,10 @@ const struct dpif_class dpif_netlink_class = { NULL, /* bond_add */ NULL, /* bond_del */ NULL, /* bond_stats_get */ + dpif_netlink_cache_get_supported_levels, + dpif_netlink_cache_get_name, + dpif_netlink_cache_get_size, + dpif_netlink_cache_set_size, }; static int @@ -4320,6 +4419,9 @@ dpif_netlink_dp_from_ofpbuf(struct dpif_netlink_dp *dp, const struct ofpbuf *buf [OVS_DP_ATTR_USER_FEATURES] = { .type = NL_A_U32, .optional = true }, + [OVS_DP_ATTR_MASKS_CACHE_SIZE] = { + .type = NL_A_U32, + .optional = true }, }; dpif_netlink_dp_init(dp); @@ -4352,6 +4454,12 @@ dpif_netlink_dp_from_ofpbuf(struct dpif_netlink_dp *dp, const struct ofpbuf *buf dp->user_features = nl_attr_get_u32(a[OVS_DP_ATTR_USER_FEATURES]); } + if (a[OVS_DP_ATTR_MASKS_CACHE_SIZE]) { + dp->cache_size = nl_attr_get_u32(a[OVS_DP_ATTR_MASKS_CACHE_SIZE]); + } else { + dp->cache_size = UINT32_MAX; + } + return 0; } @@ -4380,6 +4488,10 @@ dpif_netlink_dp_to_ofpbuf(const struct dpif_netlink_dp *dp, struct ofpbuf *buf) nl_msg_put_u32(buf, OVS_DP_ATTR_USER_FEATURES, dp->user_features); } + if (dp->cache_size != UINT32_MAX) { + nl_msg_put_u32(buf, OVS_DP_ATTR_MASKS_CACHE_SIZE, dp->cache_size); + } + /* Skip OVS_DP_ATTR_STATS since we never have a reason to serialize it. */ } @@ -4388,6 +4500,7 @@ static void dpif_netlink_dp_init(struct dpif_netlink_dp *dp) { memset(dp, 0, sizeof *dp); + dp->cache_size = UINT32_MAX; } static void diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h index 0e024c1c9..61bd90f57 100644 --- a/lib/dpif-provider.h +++ b/lib/dpif-provider.h @@ -628,6 +628,26 @@ struct dpif_class { * sufficient to store BOND_BUCKETS number of elements. */ int (*bond_stats_get)(struct dpif *dpif, uint32_t bond_id, uint64_t *n_bytes); + + /* Cache configuration + * + * Multiple levels of cache can exist in a given datapath implementation. + * An API has been provided to get the number of supported caches, which + * can then be used to get/set specific configuration. Cache level is 0 + * indexed, i.e. if 1 level is supported, the level value to use is 0. + * + * Get the number of cache levels supported. */ + int (*cache_get_supported_levels)(struct dpif *dpif, uint32_t *levels); + + /* Get the cache name for the given level. */ + int (*cache_get_name)(struct dpif *dpif, uint32_t level, + const char **name); + + /* Get currently configured cache size. */ + int (*cache_get_size)(struct dpif *dpif, uint32_t level, uint32_t *size); + + /* Set cache size. */ + int (*cache_set_size)(struct dpif *dpif, uint32_t level, uint32_t size); }; extern const struct dpif_class dpif_netlink_class; diff --git a/lib/dpif.c b/lib/dpif.c index 7cac3a629..4db897c7d 100644 --- a/lib/dpif.c +++ b/lib/dpif.c @@ -2018,3 +2018,35 @@ dpif_bond_stats_get(struct dpif *dpif, uint32_t bond_id, ? dpif->dpif_class->bond_stats_get(dpif, bond_id, n_bytes) : EOPNOTSUPP; } + +int +dpif_cache_get_supported_levels(struct dpif *dpif, uint32_t *levels) +{ + return dpif->dpif_class->cache_get_supported_levels + ? dpif->dpif_class->cache_get_supported_levels(dpif, levels) + : EOPNOTSUPP; +} + +int +dpif_cache_get_name(struct dpif *dpif, uint32_t level, const char **name) +{ + return dpif->dpif_class->cache_get_name + ? dpif->dpif_class->cache_get_name(dpif, level, name) + : EOPNOTSUPP; +} + +int +dpif_cache_get_size(struct dpif *dpif, uint32_t level, uint32_t *size) +{ + return dpif->dpif_class->cache_get_size + ? dpif->dpif_class->cache_get_size(dpif, level, size) + : EOPNOTSUPP; +} + +int +dpif_cache_set_size(struct dpif *dpif, uint32_t level, uint32_t size) +{ + return dpif->dpif_class->cache_set_size + ? dpif->dpif_class->cache_set_size(dpif, level, size) + : EOPNOTSUPP; +} diff --git a/lib/dpif.h b/lib/dpif.h index 43c1ab998..35f48bec6 100644 --- a/lib/dpif.h +++ b/lib/dpif.h @@ -905,6 +905,13 @@ int dpif_bond_del(struct dpif *, uint32_t bond_id); int dpif_bond_stats_get(struct dpif *, uint32_t bond_id, uint64_t *n_bytes); bool dpif_supports_lb_output_action(const struct dpif *); + +/* Cache */ +int dpif_cache_get_supported_levels(struct dpif *dpif, uint32_t *levels); +int dpif_cache_get_name(struct dpif *dpif, uint32_t level, const char **name); +int dpif_cache_get_size(struct dpif *dpif, uint32_t level, uint32_t *size); +int dpif_cache_set_size(struct dpif *dpif, uint32_t level, uint32_t size); + /* Miscellaneous. */ diff --git a/utilities/ovs-dpctl.c b/utilities/ovs-dpctl.c index 7d99607f4..66055cfe2 100644 --- a/utilities/ovs-dpctl.c +++ b/utilities/ovs-dpctl.c @@ -195,6 +195,10 @@ usage(void *userdata OVS_UNUSED) " get-flow [DP] ufid:UFID fetch flow corresponding to UFID\n" " del-flow [DP] FLOW delete FLOW from DP\n" " del-flows [DP] delete all flows from DP\n" + " cache-get-size [DP] " \ + "Show the current size for all caches\n" + " cache-set-size DP CACHE SIZE " \ + "Set cache size for a specific cache\n" " dump-conntrack [DP] [zone=ZONE] " \ "display conntrack entries for ZONE\n" " flush-conntrack [DP] [zone=ZONE] [ct-tuple]" \
This patch adds a general way of viewing/configuring datapath cache sizes. With an implementation for the netlink interface. The ovs-dpctl/ovs-appctl show commands will display the current cache sizes configured: ovs-dpctl show system@ovs-system: lookups: hit:25 missed:63 lost:0 flows: 0 masks: hit:282 total:0 hit/pkt:3.20 cache: hit:4 hit rate:4.5455% caches: masks cache: size: 256 port 0: ovs-system (internal) port 1: br-int (internal) port 2: genev_sys_6081 (geneve: packet_type=ptap) port 3: br-ex (internal) port 4: eth2 port 5: sw0p1 (internal) port 6: sw0p3 (internal) A specific cache can be configured as follows: ovs-appctl dpctl/cache-set-size DP CACHE SIZE ovs-dpctl cache-set-size DP CACHE SIZE For example to disable the cache do: $ ovs-dpctl cache-set-size system@ovs-system "masks cache" 0 Setting cache size successful, new size 0. Signed-off-by: Eelco Chaudron <echaudro@redhat.com> --- datapath/linux/compat/include/linux/openvswitch.h | 1 lib/dpctl.c | 120 +++++++++++++++++++++ lib/dpif-netdev.c | 4 + lib/dpif-netlink.c | 113 ++++++++++++++++++++ lib/dpif-provider.h | 20 ++++ lib/dpif.c | 32 ++++++ lib/dpif.h | 7 + utilities/ovs-dpctl.c | 4 + 8 files changed, 301 insertions(+)