On 2015/12/24 12:51, Yuanhan Liu wrote: > On Wed, Dec 23, 2015 at 11:00:15PM +0100, Thomas Monjalon wrote: >> 2015-12-23 10:44, Yuanhan Liu: >>> On Tue, Dec 22, 2015 at 01:38:29AM -0800, Rich Lane wrote: >>>> On Mon, Dec 21, 2015 at 9:47 PM, Yuanhan Liu <yuanhan.liu at >>>> linux.intel.com> >>>> wrote: >>>> >>>> On Mon, Dec 21, 2015 at 08:47:28PM -0800, Rich Lane wrote: >>>> > The queue state change callback is the one new API that needs to be >>>> > added because >>>> > normal NICs don't have this behavior. >>>> >>>> Again I'd ask, will vring_state_changed() be enough, when above issues >>>> are resolved: vring_state_changed() will be invoked at new_device()/ >>>> destroy_device(), and of course, ethtool change? >>>> >>>> >>>> It would be sufficient. It is not a great API though, because it requires >>>> the >>>> application to do the conversion from struct virtio_net to a DPDK port >>>> number, >>>> and from a virtqueue index to a DPDK queue id and direction. Also, the >>>> current >>>> implementation often makes this callback when the vring state has not >>>> actually >>>> changed (enabled -> enabled and disabled -> disabled). >>>> >>>> If you're asking about using vring_state_changed() _instead_ of the link >>>> status >>>> event and rte_eth_dev_socket_id(), >>> No, I like the idea of link status event and rte_eth_dev_socket_id(); >>> I was just wondering why a new API is needed. Both Tetsuya and I >>> were thinking to leverage the link status event to represent the >>> queue stats change (triggered by vring_state_changed()) as well, >>> so that we don't need to introduce another eth event. However, I'd >>> agree that it's better if we could have a new dedicate event. >>> >>> Thomas, here is some background for you. For vhost pmd and linux >>> virtio-net combo, the queue can be dynamically changed by ethtool, >>> therefore, the application wishes to have another eth event, say >>> RTE_ETH_EVENT_QUEUE_STATE_CHANGE, so that the application can >>> add/remove corresponding queue to the datapath when that happens. >>> What do you think of that? >> Yes it is an event. So I don't understand the question. >> What may be better than a specific rte_eth_event_type? > The alternative is a new set of callbacks, but judging that we already > have a set of callback for vhost libraray, and adding a new set to vhost > pmd doesn't seem elegant to me. > > Therefore, I'd prefer a new eth event. > > --yliu
I am ok to have one more event type. BTW, I have questions about numa_node. I guess "rte_eth_dev_socket_id()" can only return numa_node of specified port. If multiple queues are used in one device(port), can we say all queues are always in same numa_node? If the answer is no, I am still not sure we can remove "struct virtio_net" from DPDK application callback handling. I agree we can add RTE_ETH_EVENT_QUEUE_STATE_CHANGE for interrupt notice. But current ethdev APIs may not be able to hide vhost specific properties, then the callback hander needs to handle "struct virtio_net" directly. Thanks, Tetsuya
