Hi Ilya, Ilya Maximets, Jun 29, 2023 at 23:57: > > + if (flags && ovsrcu_get(void *, &netdev->hw_info.offload_data)) { > > We probbaly shouldn't use the offload_data outside of netdev-offload > modules. Simply checking netdev_is_flow_api_enabled() should be > enough. Since both features require rte_flow oflload it's unlikely > that rx-steering can work if flow api is enabled globally. And > netdev-offload-dpdk doesn't really check device capabilities.
OK, I'll change that for v13. > > static int > > netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args, > > char **errp) > > Aside from the set_config, we also need a get_config callback update. > get_config() should return everything that set_config() set. i.e. > we should return the "rx-steering": "rss+lacp", if user configured it. > The output will be visible in the dpif/show. Hmm, I had missed the get_config callback. I have added something for get status but I assume it is different. > > + memset(reta_conf, 0, sizeof(reta_conf)); > > + for (uint16_t i = 0; i < info.reta_size; i++) { > > + uint16_t idx = i / RTE_ETH_RETA_GROUP_SIZE; > > + uint16_t shift = i % RTE_ETH_RETA_GROUP_SIZE; > > An empty line here. > > > + reta_conf[idx].mask |= 1ULL << shift; > > + reta_conf[idx].reta[shift] = i % rss_n_rxq; > > + } > > And here. Ack. > > + for (int i = 0; i < dev->rx_steer_flows_num; i++) { > > + set_error(&error, RTE_FLOW_ERROR_TYPE_NONE); > > + if (rte_flow_destroy(dev->port_id, dev->rx_steer_flows[i], > > &error)) { > > + VLOG_DBG("%s: rx-steering: failed to destroy flow: %s", > > Maybe a WARN ? Will change that. > So, this function destroys all the flows, OK. > But we also need to restore the redirection table, right? Or is it handled > somehow in the driver when number of queues changed? From my experience, > drivers tend to not restore this kind of configuration even on device detach > and it gets preserved even across OVS restarts. >From what I saw during testing, the RETA is reset every time the device is reset. In the first versions of this patch, I did reset the table but I seem to remember some discussions with Kevin and/or David where we concluded that it was redundant. I will check again to make sure before sending a respin. Thanks for the review. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev