From: Sabrina Dubroca <sd@queasysnail•net>
To: Maciej Machnikowski <maciej.machnikowski@intel•com>
Cc: netdev@vger•kernel.org, intel-wired-lan@lists•osuosl.org,
richardcochran@gmail•com, abyagowi@fb•com,
anthony.l.nguyen@intel•com, davem@davemloft•net, kuba@kernel•org,
linux-kselftest@vger•kernel.org, idosch@idosch•org,
mkubecek@suse•cz, saeed@kernel•org, michael.chan@broadcom•com,
petrm@nvidia•com
Subject: Re: [PATCH v3 net-next 2/6] rtnetlink: Add new RTM_GETEECSTATE message to get SyncE status
Date: Thu, 11 Nov 2021 17:01:02 +0100 [thread overview]
Message-ID: <YY0+PmNU4MSGfgqA@hog> (raw)
In-Reply-To: <20211110114448.2792314-3-maciej.machnikowski@intel.com>
Hello Maciej,
2021-11-10, 12:44:44 +0100, Maciej Machnikowski wrote:
> diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
> index 5888492a5257..1d8662afd6bd 100644
> --- a/include/uapi/linux/rtnetlink.h
> +++ b/include/uapi/linux/rtnetlink.h
> @@ -185,6 +185,9 @@ enum {
> RTM_GETNEXTHOPBUCKET,
> #define RTM_GETNEXTHOPBUCKET RTM_GETNEXTHOPBUCKET
>
> + RTM_GETEECSTATE = 124,
> +#define RTM_GETEECSTATE RTM_GETEECSTATE
I'm not sure about this. All the other RTM_GETxxx are such that
RTM_GETxxx % 4 == 2. Following the current pattern, 124 should be
reserved for RTM_NEWxxx, and RTM_GETEECSTATE would be 126.
Also, why are you leaving a gap (which you end up filling in patch
4/6)?
> +
> __RTM_MAX,
> #define RTM_MAX (((__RTM_MAX + 3) & ~3) - 1)
> };
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 2af8aeeadadf..03bc773d0e69 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -5467,6 +5467,83 @@ static int rtnl_stats_dump(struct sk_buff *skb, struct netlink_callback *cb)
> return skb->len;
> }
>
> +static int rtnl_fill_eec_state(struct sk_buff *skb, struct net_device *dev,
> + u32 portid, u32 seq, struct netlink_callback *cb,
> + int flags, struct netlink_ext_ack *extack)
> +{
[...]
> + nlh = nlmsg_put(skb, portid, seq, RTM_GETEECSTATE, sizeof(*state_msg),
> + flags);
> + if (!nlh)
> + return -EMSGSIZE;
> +
> + state_msg = nlmsg_data(nlh);
> + state_msg->ifindex = dev->ifindex;
Why stuff this in a struct instead of using an attribute?
> +
> + if (nla_put_u32(skb, IFLA_EEC_STATE, state))
> + return -EMSGSIZE;
> +
> + if (!ops->ndo_get_eec_src)
> + goto end_msg;
> +
> + err = ops->ndo_get_eec_src(dev, &src_idx, extack);
> + if (err)
> + return err;
> +
> + if (nla_put_u32(skb, IFLA_EEC_SRC_IDX, src_idx))
> + return -EMSGSIZE;
> +
> +end_msg:
> + nlmsg_end(skb, nlh);
> + return 0;
> +}
> +
Thanks,
--
Sabrina
next prev parent reply other threads:[~2021-11-11 16:01 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-10 11:44 [PATCH v3 net-next 0/6] Add RTNL interface for SyncE Maciej Machnikowski
2021-11-10 11:44 ` [PATCH v3 net-next 1/6] ice: add support detecting features based on netlist Maciej Machnikowski
2021-11-10 11:44 ` [PATCH v3 net-next 2/6] rtnetlink: Add new RTM_GETEECSTATE message to get SyncE status Maciej Machnikowski
2021-11-11 16:01 ` Sabrina Dubroca [this message]
2021-11-11 16:22 ` Florian Westphal
2021-11-16 14:40 ` Machnikowski, Maciej
2021-11-16 15:41 ` Florian Westphal
2021-11-16 19:30 ` Jakub Kicinski
2021-11-16 14:37 ` Machnikowski, Maciej
2021-11-10 11:44 ` [PATCH v3 net-next 3/6] ice: add support for reading SyncE DPLL state Maciej Machnikowski
2021-11-10 11:44 ` [PATCH v3 net-next 4/6] rtnetlink: Add support for SyncE recovered clock configuration Maciej Machnikowski
2021-11-10 11:44 ` [PATCH v3 net-next 5/6] ice: add support for SyncE recovered clocks Maciej Machnikowski
2021-11-10 11:44 ` [PATCH v3 net-next 6/6] docs: net: Add description of SyncE interfaces Maciej Machnikowski
2021-11-11 12:43 ` Petr Machata
2021-11-15 10:24 ` Machnikowski, Maciej
2021-11-16 11:07 ` Petr Machata
2021-11-16 11:52 ` Petr Machata
2021-11-16 14:26 ` Machnikowski, Maciej
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YY0+PmNU4MSGfgqA@hog \
--to=sd@queasysnail$(echo .)net \
--cc=abyagowi@fb$(echo .)com \
--cc=anthony.l.nguyen@intel$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=idosch@idosch$(echo .)org \
--cc=intel-wired-lan@lists$(echo .)osuosl.org \
--cc=kuba@kernel$(echo .)org \
--cc=linux-kselftest@vger$(echo .)kernel.org \
--cc=maciej.machnikowski@intel$(echo .)com \
--cc=michael.chan@broadcom$(echo .)com \
--cc=mkubecek@suse$(echo .)cz \
--cc=netdev@vger$(echo .)kernel.org \
--cc=petrm@nvidia$(echo .)com \
--cc=richardcochran@gmail$(echo .)com \
--cc=saeed@kernel$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox