[PATCH 1/2] idiag: add a copy of linux/sock_diag.h

Cong Wang xiyou.wangcong at gmail.com
Fri Oct 24 09:43:25 PDT 2014


On Fri, Oct 24, 2014 at 1:40 AM, Thomas Haller <thaller at redhat.com> wrote:
> Hi Cong,
>
>
> do you mind if I split up your first commit in 3 parts?
>
>
> Also, I think, that
>
> +#if (SK_MEMINFO_VARS > SK_MEMINFO_OPTMEM + 1)
>      nl_dump(p, "\tbacklog: %d\n",
>                msg->idiag_skmeminfo[SK_MEMINFO_BACKLOG]);
> +#endif
>
> is wrong, because the preprocessor cannot see the enum values.
> I would jost drop that.
>

Good catch! I am wondering why compiler doesn't complain this.

I don't think we can drop this, otherwise we would have access
out of boundary. Probably we really need a run-time check,
that is, changing #if to if(). Of course, smart compiler should
still be able to drop that code on older kernel.

Ideally, kernel should define SK_MEMINFO_BACKLOG as
a macro so that we can just test it with #ifdef. :(

>
> For the second commit, note that include/netlink/netlink.h cannot
> include the private kernel header file. Hence, idiagnl_msg_get_tcpinfo()
> returns "struct tcp_info" as defined in <netinet/tcp.h>.


Hmm, interesting, why it needs tcp.h at all?

>
> We cannot internally use a different definition from
> include/linux-private/linux/tcp.h because then idiagnl_msg_get_tcpinfo()
> would break if <netinet/tcp.h> happens to define anything different.
>
> I think, we can just drop including the private header file.
>

tcpi_rcv_rtt is not defined on older kernel, dropping that header
will not compile.



More information about the libnl mailing list