On 09/28/2016 04:28 AM, Yuanhan Liu wrote: > On Tue, Sep 27, 2016 at 10:56:40PM +0300, Michael S. Tsirkin wrote: >> On Tue, Sep 27, 2016 at 11:11:58AM +0800, Yuanhan Liu wrote: >>> On Mon, Sep 26, 2016 at 10:24:55PM +0300, Michael S. Tsirkin wrote: >>>> On Mon, Sep 26, 2016 at 11:01:58AM -0700, Stephen Hemminger wrote: >>>>> I assume that if using Version 1 that the bit will be ignored >>> >>> Yes, but I will just quote what you just said: what if the guest >>> virtio device is a legacy device? I also gave my reasons in another >>> email why I consistently set this flag: >>> >>> - we have to return all features we support to the guest. >>> >>> We don't know the guest is a modern or legacy device. That means >>> we should claim we support both: VERSION_1 and ANY_LAYOUT. >>> >>> Assume guest is a legacy device and we just set VERSION_1 (the current >>> case), ANY_LAYOUT will never be negotiated. >>> >>> - I'm following the way Linux kernel takes: it also set both features. >>> >>> Maybe, we could unset ANY_LAYOUT when VERSION_1 is _negotiated_? >>> >>> The unset after negotiation I proposed turned out it won't work: the >>> feature is already negotiated; unsetting it only in vhost side doesn't >>> change anything. Besides, it may break the migration as Michael stated >>> below. >> >> I think the reverse. Teach vhost user that for future machine types >> only VERSION_1 implies ANY_LAYOUT. >> >> >>>> Therein lies a problem. If dpdk tweaks flags, updating it >>>> will break guest migration. >>>> >>>> One way is to require that users specify all flags fully when >>>> creating the virtio net device. >>> >>> Like how? By a new command line option? And user has to type >>> all those features? >> >> Make libvirt do this. users use management normally. those that don't >> likely don't migrate VMs. > > Fair enough. > >> >>>> QEMU could verify that all required >>>> flags are set, and fail init if not. >>>> >>>> This has other advantages, e.g. it adds ability to >>>> init device without waiting for dpdk to connect. > > Will the feature negotiation between DPDK and QEMU still exist > in your proposal? > >>>> >>>> However, enabling each new feature would now require >>>> management work. How about dpdk ships the list >>>> of supported features instead? >>>> Management tools could read them on source and destination >>>> and select features supported on both sides. >>> >>> That means the management tool would somehow has a dependency on >>> DPDK project, which I have no objection at all. But, is that >>> a good idea? >> >> It already starts the bridge somehow, does it not? > > Indeed. I was firstly thinking about reading the dpdk source file > to determine the DPDK supported feature list, with which the bind > is too tight. I later realized you may ask DPDK to provide a binary > to dump the list, or something like that. > >> >>> BTW, I'm not quite sure I followed your idea. I mean, how it supposed >>> to fix the ANY_LAYOUT issue here? How this flag will be set for >>> legacy device? >>> >>> --yliu >> >> For ANY_LAYOUT, I think we should just set in in qemu, >> but only for new machine types. > > What do you mean by "new machine types"? Virtio device with newer > virtio-spec version? > >> This addresses migration >> concerns. > > To make sure I followed you, do you mean the migration issue from > an older "dpdk + qemu" combo to a newer "dpdk + qemu" combo (that > more new features might be shipped)? > > Besides that, your proposal looks like a big work to accomplish. > Are you okay to make it simple first: set it consistently like > what Linux kernel does? This would at least make the ANY_LAYOUT > actually be enabled for legacy device (which is also the default > one that's widely used so far).
Before enabling anything by default, we should first optimize the 1 slot case. Indeed, micro-benchmark using testpmd in txonly[0] shows ~17% perf regression for 64 bytes case: - 2 descs per packet: 11.6Mpps - 1 desc per packet: 9.6Mpps This is due to the virtio header clearing in virtqueue_enqueue_xmit(). Removing it, we get better results than with 2 descs (1.20Mpps). Since the Virtio PMD doesn't support offloads, I wonder whether we can just drop the memset? -- Maxime [0]: For testing, you'll need these patches, else only first packets will use a single slot: - http://dpdk.org/dev/patchwork/patch/16222/ - http://dpdk.org/dev/patchwork/patch/16223/
