Thanks for the patch. I have some minor comments. On Wed, Jun 10, 2020 at 3:47 AM Harry van Haaren <[email protected]> wrote: the commit title should add "lookup prio set command"?
> > This commit adds a command for the dpif-netdev to set a specific > lookup function to a particular priority level. The command enables > runtime switching of the dpcls subtable lookup implementation. > > Selection is performed based on a priority. Higher priorities take > precedence, eg; priotity 5 will be selected instead of a priority 3. > > The two options available are 'autovalidator' and 'generic'. > The below command will set a new priority for the given function: > $ ovs-appctl dpif-netdev/subtable-lookup-set generic 2 add prio > > The autovalidator implementation can be selected at runtime now: > $ ovs-appctl dpif-netdev/subtable-lookup-set autovalidator 5 add prio > > Signed-off-by: Harry van Haaren <[email protected]> > > --- > > v3 > - Add automatic reprobe after changing priorities > --- Refactored from previous 1-second timeout based reprobe WIP-hack > - Add VLOG entries for changed dpcls and subtable counts > --- Also return the updated counts to the issuing command for visibility > - Clarify command by adding "prio" to the name > --- New command name is "dpif-netdev/subtable-lookup-prio-set" > --- Please note this new command change - previous command is now invalid > --- > lib/dpif-netdev.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 121 insertions(+) > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index 5e101e054..30806af16 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -258,6 +258,7 @@ struct dp_packet_flow_map { > static void dpcls_init(struct dpcls *); > static void dpcls_destroy(struct dpcls *); > static void dpcls_sort_subtable_vector(struct dpcls *); > +static uint32_t dpcls_subtable_lookup_reprobe(struct dpcls *cls); > static void dpcls_insert(struct dpcls *, struct dpcls_rule *, > const struct netdev_flow_key *mask); > static void dpcls_remove(struct dpcls *, struct dpcls_rule *); > @@ -860,6 +861,9 @@ dpif_netdev_xps_revalidate_pmd(const struct > dp_netdev_pmd_thread *pmd, > bool purge); > static int dpif_netdev_xps_get_tx_qid(const struct dp_netdev_pmd_thread *pmd, > struct tx_port *tx); > +static inline struct dpcls * > +dp_netdev_pmd_lookup_dpcls(struct dp_netdev_pmd_thread *pmd, > + odp_port_t in_port); > > static inline bool emc_entry_alive(struct emc_entry *ce); > static void emc_clear_entry(struct emc_entry *ce); > @@ -1260,6 +1264,97 @@ sorted_poll_thread_list(struct dp_netdev *dp, > *n = k; > } > > +static void > +dpif_netdev_subtable_lookup_set(struct unixctl_conn *conn, int argc, > + const char *argv[], void *aux OVS_UNUSED) > +{ > + /* This function requires 2 parameters (argv[1] and argv[2]) to execute. > + * argv[1] is subtable name > + * argv[2] is priority > + * argv[3] is the datapath name (optional if only 1 datapath exists) > + */ > + const char *func_name = argv[1]; > + > + errno = 0; > + char *err_char; > + uint32_t new_prio = strtoul(argv[2], &err_char, 10); > + if (errno != 0 || new_prio > UINT8_MAX) { > + unixctl_command_reply_error(conn, > + "error converting priority, use integer in range 0-255\n"); > + return; > + } > + > + int32_t err = dpcls_subtable_set_prio(func_name, new_prio); > + if (err) { > + unixctl_command_reply_error(conn, > + "error, subtable lookup function not found\n"); > + return; > + } > + > + /* argv[3] is optional datapath instance. If no datapath name is provided > + * and only one datapath exists, the one existing datapath is reprobed. > + */ > + ovs_mutex_lock(&dp_netdev_mutex); > + struct dp_netdev *dp = NULL; > + > + if (argc == 4) { > + dp = shash_find_data(&dp_netdevs, argv[3]); > + } else if (shash_count(&dp_netdevs) == 1) { > + dp = shash_first(&dp_netdevs)->data; > + } > + > + if (!dp) { > + ovs_mutex_unlock(&dp_netdev_mutex); > + unixctl_command_reply_error(conn, > + "please specify an existing datapath"); > + return; > + } > + > + /* Get PMD threads list, required to get DPCLS instances */ > + size_t n; > + uint32_t lookup_dpcls_changed = 0; > + uint32_t lookup_subtable_changed = 0; > + struct dp_netdev_pmd_thread **pmd_list; > + sorted_poll_thread_list(dp, &pmd_list, &n); > + > + /* take port mutex as HMAP iters over them */ > + ovs_mutex_lock(&dp->port_mutex); > + > + for (size_t i = 0; i < n; i++) { > + struct dp_netdev_pmd_thread *pmd = pmd_list[i]; > + if (pmd->core_id == NON_PMD_CORE_ID) { > + continue; > + } > + > + struct dp_netdev_port *port = NULL; > + HMAP_FOR_EACH (port, node, &dp->ports) { > + odp_port_t in_port = port->port_no; > + struct dpcls *cls = dp_netdev_pmd_lookup_dpcls(pmd, in_port); > + if (!cls) { > + continue; > + } > + uint32_t subtbl_changes = dpcls_subtable_lookup_reprobe(cls); > + if (subtbl_changes) { > + lookup_dpcls_changed++; > + lookup_subtable_changed += subtbl_changes; > + } > + } > + } > + > + /* release port mutex before netdev mutex */ > + ovs_mutex_unlock(&dp->port_mutex); > + ovs_mutex_unlock(&dp_netdev_mutex); > + > + struct ds reply = DS_EMPTY_INITIALIZER; > + ds_put_format(&reply, > + "Lookup priority change affected %d dpcls ports and %d subtables.\n", > + lookup_dpcls_changed, lookup_subtable_changed); > + const char *reply_str = ds_cstr(&reply); > + unixctl_command_reply(conn, reply_str); > + VLOG_INFO("%s", reply_str); > + ds_destroy(&reply); > +} > + > static void > dpif_netdev_pmd_rebalance(struct unixctl_conn *conn, int argc, > const char *argv[], void *aux OVS_UNUSED) > @@ -1429,6 +1524,10 @@ dpif_netdev_init(void) > "[-us usec] [-q qlen]", > 0, 10, pmd_perf_log_set_cmd, > NULL); > + unixctl_command_register("dpif-netdev/subtable-lookup-prio-set", > + "[lookup_func] [prio] [dp]", alignment, add one more space Thanks William _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
