| Message ID | 20201109151814.15040-9-kgraul@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On Mon, 9 Nov 2020 16:18:07 +0100 Karsten Graul wrote: > From: Guvenc Gulce <guvenc@linux.ibm.com> > > 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@linux.ibm.com> > Signed-off-by: Karsten Graul <kgraul@linux.ibm.com> > 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; > }
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); - + 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) + c.dump = smc_diag_dump_ext; + else + c.dump = smc_diag_dump; + return netlink_dump_start(net->diag_nlsk, skb, h, &c); } return 0; }