Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753855AbYH0Urr (ORCPT ); Wed, 27 Aug 2008 16:47:47 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1756049AbYH0Urb (ORCPT ); Wed, 27 Aug 2008 16:47:31 -0400 Received: from postel.suug.ch ([194.88.212.233]:38526 "EHLO postel.suug.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752586AbYH0Ura (ORCPT ); Wed, 27 Aug 2008 16:47:30 -0400 Date: Wed, 27 Aug 2008 22:47:50 +0200 From: Thomas Graf To: "Duyck, Alexander H" Cc: Alexander Duyck , Stephen Hemminger , "Kirsher, Jeffrey T" , "davem@davemloft.net" , "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "akpm@linux-foundation.org" Subject: Re: [PATCH] IPROUTE: correct nla nested message generated by netem_parse_opt Message-ID: <20080827204750.GV20815@postel.suug.ch> References: <20080822005011.4615.98303.stgit@jtkirshe-mobile.jf.intel.com> <20080821211941.66090862@speedy> <5f2db9d90808211937o1d9b33t9ad933938872e08d@mail.gmail.com> <20080827144122.GT20815@postel.suug.ch> <80769D7B14936844A23C0C43D9FBCF0F1506159B@orsmsx501.amr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <80769D7B14936844A23C0C43D9FBCF0F1506159B@orsmsx501.amr.corp.intel.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3012 Lines: 63 Please learn to use you enter key. Thank you. * Duyck, Alexander H 2008-08-27 09:30 > >How was it ever supposed to work? The code looked like the following > >prior to commit b03f4672007e533c8dbf0965f995182586216bf1 which made > >it used nla_parse_nested_compat() > > > >- /* Handle nested options after initial queue options. > >- * Should have put all options in nested format but > >too late now. > >- */ > >- if (nla_len(opt) > sizeof(*qopt)) { > >- struct nlattr *tb[TCA_NETEM_MAX + 1]; > >- if (nla_parse(tb, TCA_NETEM_MAX, > >- nla_data(opt) + sizeof(*qopt), > >- nla_len(opt) - sizeof(*qopt), NULL)) > > > >nla_parse_nested_compat() now does exactly what the above code does. So > >in what way has the kernel ABI changed? > > This is exactly what I am talking about. Someone assumed it was just manually coding the nested compat format and it wasn't. It was using its own custom message format. The change you made changed the kernel ABI to support that custom format instead fixing the message sent to fit the kernel ABI format. For the last time, the message format *CANNOT* be changed, the ABI has to be stable. I didn't change the ABI, I _restored the ABI to the original state after it was broken by the initial nla_parse_nested_compat() implementation which contained a bug. It's actually trivial to figure this out in 5 minutes using git-whatchanged, not sure why you didn't do it. 1092cb219774a82b1f16781aec7b8d4ec727c981 [NETLINK]: attr: add nested compat attribute type Introduced nla_parse_nested_compat() for use in netem, unfortunately it contained a bug which made it behave incorrectly and broke netem. b9a2f2e450b0f770bb4347ae8d48eb2dea701e24 netlink: Fix nla_parse_nested_compat() to call nla_parse() directly This fixed nla_parse_nested_compat() in the way it was supposed to be from the beginning. It restored the original ABI. Unfortunately, commit 1e90474c377e92db7262a8968a45c1dd980ca9e5 made prio use nla_parse_nested_compat() which it shouldn't have as it does not use the same format. It worked due to the bug in nla_parse_nested_compat() and then broke when nla_parse_nested_compat() was fixed. Since prio was removed, this is no longer a problem. Ever since, netem is the only user of nla_parse_nested_compat() so I have no idea what you mean when you say I broke it for everybody else. To put it very very simple, users of the message format as found in netem is supposed to use nla_parse_nested_compat(), everybody else is supposed to be using nla_parse_nested(). I won't bother to comment on the rest, since it shoudl be answered with this or simply didn't make any sense. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/