2016-10-28 7:45 GMT-07:00 Ciara Loftus <[email protected]>: > This RFC series consists of 3 patches. > 1. Port Hotplug (Mauricio Vasquez) (v8) > Previous: http://openvswitch.org/pipermail/dev/2016-July/075350.html > > 2. Arbitrary Port Naming (Ciara Loftus) (v4) > Previous: http://openvswitch.org/pipermail/dev/2016-July/075385.html > > 3. Experimental vDev PMD (Ciara Loftus) (v1) > Inspired by suggestions made during the v3 review: > http://openvswitch.org/pipermail/dev/2016-July/075861.html > > The version (4) is relative to the original Arbitrary port naming patch > (see link #2). > > Change log: > > v4: > - Rebase > - Include Hotplug patch > - Add vdev patch > > v3: > - remove global pci list > - remove unnecessary parenthesis > - remove return from void fn > - print pci like dpdk > - fix port ranges > > v2: > - Applied changes on top of hotplug patch and made the combination > compatible. > > > Ciara Loftus (2): > netdev-dpdk: Arbitrary 'dpdk' port naming > netdev-dpdk: Add new 'dpdkvdev' experimental port type > > Mauricio Vasquez (1): > netdev-dpdk: add hotplug support >
Thanks for the series! I tested it and I was able to add ports using the pcap and af_packet pmd, I think that's extremely useful for testing. I have a few comments: As discussed in the summit I think it's safe to merge dpdk and dpdkvdev types. I'm not too worried about dpdkvdev being more 'experimental', I think we should try to design a consistent interface. netdev_dpdk_process_vdevargs() and netdev_dpdk_process_pdevargs() need OVS_REQUIRES(dev->mutex) annotations. In the first patch, instead of using snprintf() with a fixed size buffer, how about using xasprintf()? Almost all the code introduced in the first patch is removed in the following one. I guess we could keep the detach appctl, in case someone wants to bind a device back to the kernel stack without killing ovs-vswitchd (one could argue about also keeping the attach appctl, for consistency, I'm not sure about that) netdev_dpdk_set_config() should always process dpdk-devargs even if rte_eth_dev_is_valid_port(dev->port_id) is true. We have to adapt if the user changes the dpdk-devargs option in an Interface in the database. I think we could use reconfiguration to adapt to dpdk-devargs changes. I believe that calling rte_eth_dev_stop() is not possible when traffic is flowing. netdev_dpdk_set_config() should return an error if the dpdk-devargs is not in the options or if the value is invalid or if we fail to attach, so that the error can be reported to ovs-vsctl. Lastly, I sent a series that refactors dpdk port creation and postpones initialization to the first reconfiguration (until commit 6), maybe it can make this patch easier: https://mail.openvswitch.org/pipermail/ovs-dev/2016-November/325141.html Thanks, Daniele _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
