From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Ahern Subject: Re: [PATCH iproute2] devlink: Ignore unknown attributes Date: Fri, 19 Jan 2018 16:00:36 -0800 Message-ID: <02ffb733-8a2d-2ad0-2643-d3ef88cf8856@gmail.com> References: <1516195680-33683-1-git-send-email-arkadis@mellanox.com> <695fecfe-c45e-4a83-cee8-0476ee56d422@gmail.com> <20180119140232.1191999c@xeon-e3> <0bb82a68-bc94-0c9e-feaa-f2801063ab63@gmail.com> <20180119154031.5a16e1a1@xeon-e3> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: Arkadi Sharshevsky , netdev@vger.kernel.org, davem@davemloft.net, mlxsw@mellanox.com To: Stephen Hemminger Return-path: Received: from mail-pg0-f49.google.com ([74.125.83.49]:39588 "EHLO mail-pg0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753054AbeATAAj (ORCPT ); Fri, 19 Jan 2018 19:00:39 -0500 Received: by mail-pg0-f49.google.com with SMTP id w17so2571802pgv.6 for ; Fri, 19 Jan 2018 16:00:39 -0800 (PST) In-Reply-To: <20180119154031.5a16e1a1@xeon-e3> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 1/19/18 3:40 PM, Stephen Hemminger wrote: >> mnl_attr_type_valid calls mnl_attr_get_type which does attr->nla_type & >> NLA_TYPE_MASK. Since you are no longer acknowledging the return code of >> mnl_attr_type_valid, you don't care about its checks so you might as >> well not call it. I don't see anything in libmnl that checks that >> mnl_attr_type_valid is invoked on an attr, so hence my question -- given >> the change above why call it all? ok. I see the error in my thinking. > The part that matters is: > > static int attr_cb(const struct nlattr *attr, void *data) > { > const struct nlattr **tb = data; > int type; > > if (mnl_attr_type_valid(attr, DEVLINK_ATTR_MAX) < 0) << makes sure that type < DEVLINK_ATTR_MAX > return MNL_CB_OK; > > type = mnl_attr_get_type(attr); > if (mnl_attr_validate(attr, devlink_policy[type]) < 0) << this part doesn't matter really > return MNL_CB_ERROR; > > tb[type] = attr; << necessary so that tb[] is filled in. > return MNL_CB_OK; > }