Re: Netlink Connector / CBUS

[Evgeniy Polyakov]

On Tue, 2005-04-05 at 01:05 -0400, James Morris wrote: 
> Evgeniy,
> 
> Please send networking patches to netdev@xxxxxxxxxxxx

It was sent there two times.

> Your connector code (under drivers/connector) is now in the -mm tree and 
> as far as I can tell, has not received any review from the network 
> developers.

I received comments and feature requests from Herbert Xu and Jamal Hadi
Salim,
almost all were successfully resolved.

> Looking at it briefly, it seems quite unfinished.

Hmmm...
I think it is fully functional and ready for inclusion.

> I'm not entirely sure what it's purpose is.

1. Provide very flexible userspace control over netlink.
2. Provide very flexible notification mechanism.

> A clear explanation of its purpose would be helpful (to me, at least), as 
> well as documentation of the API and majore data structures (which akpm 
> has also asked for, IIRC).

Documentation exists in Documentation/connector/connector.txt.
Patch with brief source documentation was already created,
so I will post it with other minor updates soon.

> I can see one example of where it's being used with kobject_uevent, and it 
> seems to have arrived via Greg-KH's I2C tree...

It also is used in SuperIO and acrypto subsystems.

> If you're trying to add a generic, psuedo-reliable Netlink communication 
> system, perhaps this should be built into Netlink itself as an extension 
> of the existing Netlink API.

So, you recommend to create for each driver, that wants to be controlled
over netlink, new netlink socket, register it's unit and learn
how SKB is allocated, processed and so on?
This is wrong.
Much easier to just register a callback.

> I don't think this should be done as a separate "driver" off somewhere 
> else with a new API.

It is much easier to use connector instead of direct netlink sockets.
One should only register callback and identifier. When driver receives
special netlink message with appropriate identifier, appropriate
callback will be called.

From the userspace point of view it's quite straightforward:
socket();
bind();
send();
recv();

But if kernelspace want to use full power of such connections, driver
writer must create special sockets, must know about struct sk_buff
handling...
Connector allows any kernelspace agents to use netlink based
networking for inter-process communication in a significantly easier
way:

int cn_add_callback(struct cb_id *id, char *name, void (*callback) (void
*));
void cn_netlink_send(struct cn_msg *msg, u32 __groups);


> 
> A few questions:
> 
> - Why does it by default use NETLINK_NFLOG a kernel socket, and also allow 
> this to be overriden by a module parameter?

Because while this driver lived outside kernel tree there were no empty 
registered socket.
It can be changed if driver will go upstream.

> - Why does the cn.o module (poor namespace choice) add a callback itself
> on initialization?

Because that callback is used for notification requests.

> - Where is the userspace code which uses this?  I checked out dbus from 
> cvs and couldn't see anything obvious.

I posted it with SuperIO, kobject_uevent, acrypto and fork changes.
It is quite straightforward:

        s = socket(PF_NETLINK, SOCK_DGRAM, NETLINK_NFLOG);
        if (s == -1) {
                perror("socket");
                return -1;
        }

        l_local.nl_family = AF_NETLINK;
        l_local.nl_groups = CN_ACRYPTO_IDX;
        l_local.nl_pid = getpid();

        if (bind(s, (struct sockaddr *)&l_local, sizeof(struct
sockaddr_nl)) == -1) {
                perror("bind");
                close(s);
                return -1;
        }


                case NLMSG_DONE:
                        data = (struct cn_msg *)NLMSG_DATA(reply);
                        m = (struct crypto_conn_data *)(data + 1);
                        stat = (struct crypto_device_stat *)(m+1);

                        time(&tm);
                        fprintf(out,
                                "%.24s : [%x.%x] [seq=%u, ack=%u], name=
%s, cmd=%#02x, "
                                "sesions: completed=%llu, started=%llu,
finished=%llu, cache_failed=%llu.\n",
                                ctime(&tm), data->id.idx, data->id.val,
                                data->seq, data->ack, m->name, m->cmd,
                                stat->scompleted, stat->sstarted, stat-
>sfinished, stat->cache_failed);
                        fflush(out);
                        break;


> 
> Thanks,

Thank you for your comments.

> 
> - James
-- 
        Evgeniy Polyakov

Crash is better than data corruption -- Arthur Grabowski

Attachment: signature.asc
Description: This is a digitally signed message part