netdev - Re: [PATCH 2/2] bna: Add Generic Netlink Interface

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BANLkTin+kRsGFWG9N16E9Red5SM=g+haSw@mail.gmail.com>
Date:	Wed, 18 May 2011 15:28:04 -0700
From:	Debashis Dutt <debdut@...il.com>
To:	David Lamparter <equinox@...c24.net>
Cc:	Rasesh Mody <rmody@...cade.com>, davem@...emloft.net,
	netdev@...r.kernel.org, adapter_linux_open_src_team@...cade.com,
	Debashis Dutt <ddutt@...cade.com>
Subject: Re: [PATCH 2/2] bna: Add Generic Netlink Interface

On Wed, May 18, 2011 at 5:00 AM, David Lamparter <equinox@...c24.net> wrote:
> On Tue, May 17, 2011 at 09:57:01PM -0700, Rasesh Mody wrote:
>> This patch adds the generic netlink communication interface to BNA driver. The
>> in-kernel generic netlink infrastructure can be used to collect hardware
>> specific control information and control attributes. The driver makes use of
>> the "doit" handler provided by the generic netlink layer to accomplish this.
> [...]
>> +struct bnad_genl_ioc_info {
>> +     int             status;
>> +     u16             bnad_num;
>> +     u16             rsvd;
>> +     char            serialnum[64];
>> +     char            hwpath[BFA_STRING_32];
>> +     char            adapter_hwpath[BFA_STRING_32];
>> +     char            guid[BFA_ADAPTER_SYM_NAME_LEN*2];
>> +     char            name[BFA_ADAPTER_SYM_NAME_LEN];
>> +     char            port_name[BFA_ADAPTER_SYM_NAME_LEN];
>> +     char            eth_name[BFA_ADAPTER_SYM_NAME_LEN];
>> +     u64             rsvd1[4];
>> +     mac_t           mac;
>> +     mac_t           factory_mac;    /* Factory mac address */
>> +     mac_t           current_mac;    /* Currently assigned mac address */
>> +     enum bfa_ioc_type     ioc_type;
>> +     u16             pvid;           /* Port vlan id */
>> +     u16             rsvd2;
>> +     u32             host;
>> +     u32             bandwidth;      /* For PF support */
>> +     u32             rsvd3;
>> +};
>> +
>> +struct bnad_genl_ioc_attr {
>> +     int             status;
>> +     u16             bnad_num;
>> +     u16             rsvd;
>> +     struct bfa_ioc_attr  ioc_attr;
>> +};
>
> These things all look like they're better put into sysfs. Why would you
> create a genl protocol just to query some presumably static attributes?
>
>
> -David
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Hi David,

There could be different ways of doing it, but the reason we chose to go
the generic netlink route are as follows:

1) The recommended format of sysfs (as per sysfs.txt in kernel documentation)
   is one single line of ASCII text per file.

   As a result this:
   a) if there are a lot of attributes to be collected,
         the driver will end up in creating that many sysfs files.
   b) Reading / writing each attribute will result in a set of
        [open()/read()/write()/close()] calls.

   This is not very scalable, particularly if we want to expand on this
   interface for better management functionality in the future.

   Instead, generic netlink provides a much better way of multiplexing data
   over a single socket.

2) Asynchronous event notifications can be much easily handled using the generic
     netlink interface.

Thanks
--Debashis
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html