Thanks for figuring this out! I guess that we can drop this patch, then. (Let me know if you still want it.)
On Thu, Jul 02, 2015 at 12:13:25AM +0000, Alin Serdean wrote: > Sorry was relocating home. > > Thanks for raising the question, it made me realize another problem :) > > To answer your question directly: no this patch just ignores the attribute > completely it does not do any additional operations. > > I introduced the attribute because it was failing at > https://github.com/openvswitch/ovs/blob/master/datapath-windows/ovsext/User.c#L341, > and to be more direct because the optional parameter was zero but apparently > this is just the symptom. > > Due to your question I relooked over the code and found the actual problem: > when we are calling NlAttrParse we are calling it with array size of the > __OVS_PACKET_ATTR_MAX(the one defined in openvswitch.h) when we should call > it with the array size of the policy itself. > > I will look over the windows datapath code to see if we slipped in that > mistake in other parts as well and will try to send the patches asap. > > Regarding the patch it is semi dead weight because it just introduces the > attribute but no real logic is behind (it will work with and without it). > > Alin. > > -----Mesaj original----- > De la: Ben Pfaff [mailto:[email protected]] > Trimis: Thursday, July 2, 2015 2:36 AM > Către: Eitan Eliahu > Cc: Alin Serdean; [email protected] > Subiect: Re: [ovs-dev] [PATCH] datapath-windows: OVS_PACKET_ATTR_PROBE > > Alin? > > On Wed, Jul 01, 2015 at 11:04:57PM +0000, Eitan Eliahu wrote: > > Hi Ben, > > In general it doesn't seem that if the datapath does not support an > > attribute it would fail. (In other words the parsing validation code will > > not fail if an attribute is not included in the policy array). > > I am not sure if the addition of this specific attribute to the policy > > masks an issue or not. > > Perhaps Alin can tell. > > Thanks, > > Eitan > > > > -----Original Message----- > > From: dev [mailto:[email protected]] On Behalf Of Ben Pfaff > > Sent: Wednesday, July 01, 2015 2:53 PM > > To: Alin Serdean > > Cc: [email protected] > > Subject: Re: [ovs-dev] [PATCH] datapath-windows: OVS_PACKET_ATTR_PROBE > > > > On Wed, Jul 01, 2015 at 06:57:50PM +0000, Alin Serdean wrote: > > > Since commit: > > > https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_open > > > vs > > > witch_ovs_commit_2e460098bff351b9fddcb917447caa3b97a35d86&d=BQIGaQ&c > > > =S > > > qcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ > > > 4e > > > 3geGYp56lkcH-5cLCY&m=wSN1s5IDTOoE-dDGQWtmN87LEkQnYq71hKWcn7d3SJk&s=8 > > > _A Va78P_KXRWsNoVVtFSjNbBrJjyie8OjjjH4U953U&e= > > > a new packet attribute was introduced. > > > > > > This patch adds OVS_PACKET_ATTR_PROBE to nlPktExecPolicy in > > > datapath-windows and ignores it for the moment to maintain binary > > > compatibility. > > > > > > Signed-off-by: Alin Gabriel Serdean > > > <[email protected]> > > > Acked-by: Eitan Eliahu <[email protected]> > > > --- > > > This patch should be applied on master and branch-2.4 > > > v2: add acked-by > > > > This patch makes me worry a bit. The Linux kernel and OVS userspace > > implementations of Netlink ignore attributes that they do not recognize, > > which allows for some kinds of compatibility. To me, this patch implies > > that the Windows datapath reports an error when it sees an unrecognized > > attribute. Is that correct? If so, then that should be changed, because > > otherwise it will cause compatibility problems in the future. > > _______________________________________________ > > dev mailing list > > [email protected] > > https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_ma > > ilman_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt- > > uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=wSN1s5IDTOoE-dDGQW > > tmN87LEkQnYq71hKWcn7d3SJk&s=TZgQzcgvt8NDu-lf_W0nI9BkcWxoExr-9T2PSSonAb > > k&e= _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
