> -----Original Message----- > From: Thomas Monjalon [mailto:tho...@monjalon.net] > Sent: Wednesday, September 13, 2017 2:21 PM > To: Ananyev, Konstantin <konstantin.anan...@intel.com>; > step...@networkplumber.org > Cc: dev@dpdk.org; Shahaf Shuler <shah...@mellanox.com> > Subject: Re: [dpdk-dev] [PATCH 4/4] ethdev: add helpers to move to the new > offloads API > > 13/09/2017 14:56, Ananyev, Konstantin: > > From: Thomas Monjalon [mailto:tho...@monjalon.net] > > > 13/09/2017 13:16, Shahaf Shuler: > > > > Wednesday, September 13, 2017 12:28 PM, Thomas Monjalon: > > > > > I still think we must streamline ethdev API instead of complexifying. > > > > > We should drop the big "configure everything" and configure offloads > > > > > one by > > > > > one, and per queue (the finer grain). > > > > > > > > The issue is, that there is some functionality which cannot be achieved > > > > when configuring offload per queue. > > > > For example - vlan filter on intel NICs. The PF can set it even without > > > > creating a single queue, in order to enable it for the VFs. > > > > > > As it is a device-specific - not documented - side effect, > > > I won't consider it. > > > > Hmm, are you saying that if there are gaps in our documentation it ok to > > break things? > > If it is not documented, we did not explicitly agree on it. > How an application knows that setting a PF settings will have > effect on related VFs? > > > Once again - you suggest to break existing functionality without providing > > any > > alternative way to support it. > > It is not a functionality, it is a side effect.
I wouldn't agree with that. DPDK does support PF for these devices. It is responsibility of PF to provide to user ability to configure and control it's VF(s). > What happens if a VF changes this settings? error? Depends on particular offload and HW. For ixgbe and igb for most cases VF is simply not physically capable to change these things. I think, that in some places error is returned, in other such request is silently ignored. > Is this error documented? > > > Surely I will NACK such proposal. > > Nothing to nack, I agree with v3 which doesn't break ixgbe VLAN settings. Ok then. > > Konstantin, I would like your opinion about the proposal below. > It is about making on the fly configuration more generic. > You say it is possible to configure VLAN on the fly, > and I think we should make it possible for other offload features. It would be a good thing, but I don't think it is possible for all offloads. For some of them you still have to stop the queue(port) first. Also I am not sure what exactly do you propose? Is that something like that: - wipe existing offload bitfileds from rte_eth_rxmode (already done by Shahaf) - Instead of uint64_t offloads inside both rte_eth_rxmode and te_eth_rxconf Introduce new functions: int rte_eth_set_port_rx_offload(portid, uint64_t offload_mask); int rte_eth_set_queue_rx_offload(portid, queueid, uint64_t offload_mask); uint64_t rte_eth_get_port_rx_offload(portid); uint64_t rte_eth_set_queue_rx_offload(portid, queueid); And add new fileds: rx_offload_port_dynamic_capa rx_offload_queue_dynamic_capa inside rte_eth_dev_info. And it would be user responsibility to call set_port/queue_rx_offload() somewhere before dev_start() for static offloads. ? If so, then it seems reasonable to me. Konstantin > > > > However I understand it may be better to be able to configure > > > per-port offloads with a dedicated per-port function. > > > I agree with the approach of the v3 of this series. > > > > > > Let me give my overview of offloads: > > > > > > We have simple offloads which are configured by just setting a flag. > > > The same flag can be set per-port or per-queue. > > > This offload can be set before starting or on the fly. > > > We currently have no generic way to set it on the fly. > > > > > > We have also more complicate offloads which require more configuration. > > > They are set with the rte_flow API. > > > They can be per-port, per-queue, on the fly or not (AFAIK). > > > > > > I think we must discuss "on the fly" capability. > > > It requires probably to set up simple offloads (flags) with a dedicated > > > function instead of using "configure" and "queue_setup" functions. > > > This new capability can be implemented in a different series. > > > > > > Opinions?
