[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87zgdizvfq.fsf@nvidia.com>
Date: Wed, 26 Oct 2022 13:06:13 +0200
From: Petr Machata <petrm@...dia.com>
To: Daniel Machon <daniel.machon@...rochip.com>
CC: <netdev@...r.kernel.org>, <davem@...emloft.net>,
<petrm@...dia.com>, <maxime.chevallier@...tlin.com>,
<thomas.petazzoni@...tlin.com>, <edumazet@...gle.com>,
<kuba@...nel.org>, <pabeni@...hat.com>,
<lars.povlsen@...rochip.com>, <Steen.Hegelund@...rochip.com>,
<UNGLinuxDriver@...rochip.com>, <joe@...ches.com>,
<linux@...linux.org.uk>, <horatiu.vultur@...rochip.com>,
<Julia.Lawall@...ia.fr>, <vladimir.oltean@....com>,
<linux-kernel@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [net-next v3 2/6] net: dcb: add new apptrust attribute
Daniel Machon <daniel.machon@...rochip.com> writes:
> Add new apptrust extension attributes to the 8021Qaz APP managed object.
>
> Two new attributes, DCB_ATTR_DCB_APP_TRUST_TABLE and
> DCB_ATTR_DCB_APP_TRUST, has been added. Trusted selectors are passed in
> the nested attribute DCB_ATTR_DCB_APP_TRUST, in order of precedence.
>
> The new attributes are meant to allow drivers, whose hw supports the
> notion of trust, to be able to set whether a particular app selector is
> trusted - and in which order.
>
> Signed-off-by: Daniel Machon <daniel.machon@...rochip.com>
> ---
> include/net/dcbnl.h | 4 ++
> include/uapi/linux/dcbnl.h | 10 +++++
> net/dcb/dcbnl.c | 77 ++++++++++++++++++++++++++++++++++++--
> 3 files changed, 87 insertions(+), 4 deletions(-)
>
> diff --git a/include/net/dcbnl.h b/include/net/dcbnl.h
> index 2b2d86fb3131..8841ab6c2de7 100644
> --- a/include/net/dcbnl.h
> +++ b/include/net/dcbnl.h
> @@ -109,6 +109,10 @@ struct dcbnl_rtnl_ops {
> /* buffer settings */
> int (*dcbnl_getbuffer)(struct net_device *, struct dcbnl_buffer *);
> int (*dcbnl_setbuffer)(struct net_device *, struct dcbnl_buffer *);
> +
> + /* apptrust */
> + int (*dcbnl_setapptrust)(struct net_device *, u8 *, int);
> + int (*dcbnl_getapptrust)(struct net_device *, u8 *, int *);
> };
>
> #endif /* __NET_DCBNL_H__ */
> diff --git a/include/uapi/linux/dcbnl.h b/include/uapi/linux/dcbnl.h
> index dc7ef96207ca..9344e3ba5768 100644
> --- a/include/uapi/linux/dcbnl.h
> +++ b/include/uapi/linux/dcbnl.h
> @@ -410,6 +410,7 @@ enum dcbnl_attrs {
> * @DCB_ATTR_IEEE_PEER_ETS: peer ETS configuration - get only
> * @DCB_ATTR_IEEE_PEER_PFC: peer PFC configuration - get only
> * @DCB_ATTR_IEEE_PEER_APP: peer APP tlv - get only
> + * @DCB_ATTR_DCB_APP_TRUST_TABLE: selector trust table
> */
> enum ieee_attrs {
> DCB_ATTR_IEEE_UNSPEC,
> @@ -423,6 +424,7 @@ enum ieee_attrs {
> DCB_ATTR_IEEE_QCN,
> DCB_ATTR_IEEE_QCN_STATS,
> DCB_ATTR_DCB_BUFFER,
> + DCB_ATTR_DCB_APP_TRUST_TABLE,
> __DCB_ATTR_IEEE_MAX
> };
> #define DCB_ATTR_IEEE_MAX (__DCB_ATTR_IEEE_MAX - 1)
> @@ -435,6 +437,14 @@ enum ieee_attrs_app {
> };
> #define DCB_ATTR_IEEE_APP_MAX (__DCB_ATTR_IEEE_APP_MAX - 1)
>
> +enum dcbnl_attrs_apptrust {
> + DCB_ATTR_DCB_APP_TRUST_UNSPEC,
> + DCB_ATTR_DCB_APP_TRUST,
> + __DCB_ATTR_DCB_APP_TRUST_MAX
> +};
> +
> +#define DCB_ATTR_DCB_APP_TRUST_MAX (__DCB_ATTR_DCB_APP_TRUST_MAX - 1)
> +
> /**
> * enum cee_attrs - CEE DCBX get attributes.
> *
> diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c
> index 92c32bc11374..01310edf6d1b 100644
> --- a/net/dcb/dcbnl.c
> +++ b/net/dcb/dcbnl.c
> @@ -166,6 +166,7 @@ static const struct nla_policy dcbnl_ieee_policy[DCB_ATTR_IEEE_MAX + 1] = {
> [DCB_ATTR_IEEE_QCN] = {.len = sizeof(struct ieee_qcn)},
> [DCB_ATTR_IEEE_QCN_STATS] = {.len = sizeof(struct ieee_qcn_stats)},
> [DCB_ATTR_DCB_BUFFER] = {.len = sizeof(struct dcbnl_buffer)},
> + [DCB_ATTR_DCB_APP_TRUST_TABLE] = {.type = NLA_NESTED},
> };
>
> /* DCB number of traffic classes nested attributes. */
> @@ -1057,11 +1058,11 @@ static int dcbnl_build_peer_app(struct net_device *netdev, struct sk_buff* skb,
> /* Handle IEEE 802.1Qaz/802.1Qau/802.1Qbb GET commands. */
> static int dcbnl_ieee_fill(struct sk_buff *skb, struct net_device *netdev)
> {
> - struct nlattr *ieee, *app;
> - struct dcb_app_type *itr;
> const struct dcbnl_rtnl_ops *ops = netdev->dcbnl_ops;
> + struct nlattr *ieee, *app, *apptrust;
> + struct dcb_app_type *itr;
> + int err, i;
> int dcbx;
> - int err;
>
> if (nla_put_string(skb, DCB_ATTR_IFNAME, netdev->name))
> return -EMSGSIZE;
> @@ -1161,6 +1162,24 @@ static int dcbnl_ieee_fill(struct sk_buff *skb, struct net_device *netdev)
> spin_unlock_bh(&dcb_lock);
> nla_nest_end(skb, app);
>
> + if (ops->dcbnl_getapptrust) {
> + u8 selectors[IEEE_8021QAZ_APP_SEL_MAX + 1] = {0};
> + int nselectors;
> +
> + apptrust = nla_nest_start(skb, DCB_ATTR_DCB_APP_TRUST_TABLE);
> + if (!app)
> + return -EMSGSIZE;
> +
> + err = ops->dcbnl_getapptrust(netdev, selectors, &nselectors);
> + if (err)
> + return -EMSGSIZE;
This should return the error coming from the driver instead of
-EMSGSIZE.
Also, it should cancel the nest before returning.
> +
> + for (i = 0; i < nselectors; i++)
> + nla_put_u8(skb, DCB_ATTR_DCB_APP_TRUST, selectors[i]);
> +
> + nla_nest_end(skb, apptrust);
> + }
> +
> /* get peer info if available */
> if (ops->ieee_peer_getets) {
> struct ieee_ets ets;
> @@ -1454,8 +1473,8 @@ static int dcbnl_ieee_set(struct net_device *netdev, struct nlmsghdr *nlh,
> {
> const struct dcbnl_rtnl_ops *ops = netdev->dcbnl_ops;
> struct nlattr *ieee[DCB_ATTR_IEEE_MAX + 1];
> + int err, i;
> int prio;
> - int err;
I don't really mind, but I have to wonder why this new variable is not
with the rest of them in the trust table scope.
>
> if (!ops)
> return -EOPNOTSUPP;
> @@ -1541,6 +1560,56 @@ static int dcbnl_ieee_set(struct net_device *netdev, struct nlmsghdr *nlh,
> }
> }
>
> + if (ieee[DCB_ATTR_DCB_APP_TRUST_TABLE]) {
> + u8 selectors[IEEE_8021QAZ_APP_SEL_MAX + 1] = {0};
> + struct nlattr *attr;
> + int nselectors = 0;
> + u8 selector;
> + int rem;
> +
> + if (!ops->dcbnl_setapptrust) {
> + err = -EOPNOTSUPP;
> + goto err;
> + }
> +
> + nla_for_each_nested(attr, ieee[DCB_ATTR_DCB_APP_TRUST_TABLE],
> + rem) {
> + if (nla_type(attr) != DCB_ATTR_DCB_APP_TRUST ||
> + nla_len(attr) != 1 ||
> + nselectors >= sizeof(selectors)) {
> + err = -EINVAL;
> + goto err;
> + }
> +
> + selector = nla_get_u8(attr);
> + switch (selector) {
> + case IEEE_8021QAZ_APP_SEL_ETHERTYPE:
> + case IEEE_8021QAZ_APP_SEL_STREAM:
> + case IEEE_8021QAZ_APP_SEL_DGRAM:
> + case IEEE_8021QAZ_APP_SEL_ANY:
> + case IEEE_8021QAZ_APP_SEL_DSCP:
> + case DCB_APP_SEL_PCP:
> + break;
> + default:
> + err = -EINVAL;
> + goto err;
> + }
> + /* Duplicate selector ? */
> + for (i = 0; i < nselectors; i++) {
> + if (selectors[i] == selector) {
> + err = -EINVAL;
> + goto err;
> + }
> + }
> +
> + selectors[nselectors++] = selector;
> + }
> +
> + err = ops->dcbnl_setapptrust(netdev, selectors, nselectors);
> + if (err)
> + goto err;
> + }
> +
> err:
> err = nla_put_u8(skb, DCB_ATTR_IEEE, err);
> dcbnl_ieee_notify(netdev, RTM_SETDCB, DCB_CMD_IEEE_SET, seq, 0);
Powered by blists - more mailing lists