From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Graf Subject: Re: [ovs-dev] [PATCH] linux: Signal datapath that unaligned Netlink message can be received Date: Fri, 15 Nov 2013 10:47:13 +0100 Message-ID: <5285EDA1.5060609@redhat.com> References: <51e10053434e3afdf9940d9be7d9f0ff4788d1f7.1384184155.git.tgraf@redhat.com> <20131111155014.GC32602@nicira.com> <5280FD6D.9000609@redhat.com> <52834A8D.9040009@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: Ben Pfaff , "dev@openvswitch.org" , Ben Hutchings , fleitner@redhat.com, dborkmann@redhat.com, netdev To: Jesse Gross Return-path: Received: from mx1.redhat.com ([209.132.183.28]:19310 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752653Ab3KOJrX (ORCPT ); Fri, 15 Nov 2013 04:47:23 -0500 In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On 11/15/2013 10:32 AM, Jesse Gross wrote: > On Wed, Nov 13, 2013 at 5:46 PM, Thomas Graf wrote: >> On 11/13/2013 07:11 AM, Jesse Gross wrote: >>> >>> On Mon, Nov 11, 2013 at 11:53 PM, Thomas Graf wrote: >>>> >>>> On 11/11/2013 04:50 PM, Ben Pfaff wrote: >>>>> >>>>> >>>>> On Mon, Nov 11, 2013 at 04:36:24PM +0100, Thomas Graf wrote: >>>>>> >>>>>> >>>>>> Following commit (''netlink: Do not enforce alignment of last Netlink >>>>>> attribute''), signal the ability to receive unaligned Netlink messages >>>>>> to the datapath to enable utilization of zerocopy optimizations. >>>>>> >>>>>> Signed-off-by: Thomas Graf >>>>> >>>>> >>>>> >>>>> Seems OK from a userspace point of view. I am a little concerned that >>>>> downgrading userspace without deleting and re-creating the datapath >>>>> (e.g. via "force-reload-kmod") will result in a totally broken setup >>>>> since userspace will then drop every packet from the kernel. >>>> >>>> >>>> >>>> Is that something that occurs occasionally in installations? Utilizing >>>> the version field in the genl header could be used to track this and >>>> clear user_features. >>> >>> >>> It's probably a good idea. I could see us having more of these >>> features flags in the future (although obviously we should try to >>> avoid them if possible) and, as Ben said, it would potentially lead to >>> a bad state otherwise. >>> >>> I'm not sure exactly what you have in mind though, can you elaborate a >>> little? >> >> >> My initial thought was to use a version field to notice the replacement >> of user space. On second thought that is not required, modifying user >> space to provide the user features in OVS_DP_CMD_GET as well will >> overwrite the features. Resetting user_features to 0 if not features are >> provided will provide backwards compatibility to versions not aware of >> user features yet. Thoughts? > > Hmm, it doesn't really seem ideal to make DP_CMD_GET change settings > as it's probably not the expected behavior for most users of the > command. One example of where this could be a problem is if it is > called from both ovs-dpctl and ovs-vswitch. Usually these would be > have the same capabilities but I'm not sure that it's strictly > required in all cases. DP_CMD_SET seems like the ideal place to put it > but I guess we don't use that at all on existing versions of OVS > userspace. Agreed. Ideal and clean is a DP_CMD_SET lookup-or-create that updates the user features. That will leave one OVS version to be non backwards compatible in terms of downgrading user features. For that reason we can bump OVS_DATAPATH_VERSION to version 2 and reset the user features if we receive a DP_CMD_GET version 1. It's not a problem if we reset the user features unnecessarily in some corner cases such as mismatching ovs-dpctl and vswitchd. The next iteration of the patchset has support for memory mapped netlink i/o which would still apply. If you agree to this then I will cook this up.