On Mon, 9 Nov 2020 16:18:07 +0100 Karsten Graul wrote:
> From: Guvenc Gulce <guvenc@xxxxxxxxxxxxx>
>
> smc_diag module should be able to work with legacy and
> extended netlink api. This is done by using the sequence field
> of the netlink message header. Sequence field is optional and was
> filled with a constant value MAGIC_SEQ in the current
> implementation.
> New constant values MAGIC_SEQ_V2 and MAGIC_SEQ_V2_ACK are used to
> signal the usage of the new Netlink API between userspace and
> kernel.
>
> Signed-off-by: Guvenc Gulce <guvenc@xxxxxxxxxxxxx>
> Signed-off-by: Karsten Graul <kgraul@xxxxxxxxxxxxx>
> diff --git a/include/uapi/linux/smc_diag.h b/include/uapi/linux/smc_diag.h
> index 8cb3a6fef553..236c1c52d562 100644
> --- a/include/uapi/linux/smc_diag.h
> +++ b/include/uapi/linux/smc_diag.h
> @@ -6,6 +6,13 @@
> #include <linux/inet_diag.h>
> #include <rdma/ib_user_verbs.h>
>
> +/* Sequence numbers */
> +enum {
> + MAGIC_SEQ = 123456,
> + MAGIC_SEQ_V2,
> + MAGIC_SEQ_V2_ACK,
> +};
> +
> /* Request structure */
> struct smc_diag_req {
> __u8 diag_family;
> diff --git a/net/smc/smc_diag.c b/net/smc/smc_diag.c
> index 44be723c97fe..bc2b616524ff 100644
> --- a/net/smc/smc_diag.c
> +++ b/net/smc/smc_diag.c
> @@ -293,19 +293,24 @@ static int smc_diag_dump(struct sk_buff *skb, struct netlink_callback *cb)
> return skb->len;
> }
>
> +static int smc_diag_dump_ext(struct sk_buff *skb, struct netlink_callback *cb)
> +{
> + return skb->len;
> +}
> +
> static int smc_diag_handler_dump(struct sk_buff *skb, struct nlmsghdr *h)
> {
> struct net *net = sock_net(skb->sk);
> -
Why did you drop the new line separating variables from code?
> + struct netlink_dump_control c = {
> + .min_dump_alloc = SKB_WITH_OVERHEAD(32768),
> + };
> if (h->nlmsg_type == SOCK_DIAG_BY_FAMILY &&
> h->nlmsg_flags & NLM_F_DUMP) {
> - {
> - struct netlink_dump_control c = {
> - .dump = smc_diag_dump,
> - .min_dump_alloc = SKB_WITH_OVERHEAD(32768),
> - };
> - return netlink_dump_start(net->diag_nlsk, skb, h, &c);
> - }
> + if (h->nlmsg_seq >= MAGIC_SEQ_V2)
This is not checked by the kernel, how do you know all user space
currently passes 123456?
Also, obviously, this is a rather weird abuse of sequence numbers.
Why don't you just add new attributes for new stuff you want to expose?
That's never mentioned anywhere, AFAICS.
> + c.dump = smc_diag_dump_ext;
> + else
> + c.dump = smc_diag_dump;
> + return netlink_dump_start(net->diag_nlsk, skb, h, &c);
> }
> return 0;
> }
