Evgeniy,
Please send networking patches to [email protected].
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.
Looking at it briefly, it seems quite unfinished.
I'm not entirely sure what it's purpose is.
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).
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...
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.
I don't think this should be done as a separate "driver" off somewhere
else with a new API.
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?
- Why does the cn.o module (poor namespace choice) add a callback itself
on initialization?
- Where is the userspace code which uses this? I checked out dbus from
cvs and couldn't see anything obvious.
Thanks,
- James
--
James Morris
<[email protected]>
On Tue, 5 Apr 2005, James Morris wrote:
> A few questions:
Also, please allow cn_add_callback() allow it to be passed a NULL
callback function, so the caller doesn't pass in a dummy function and your
code doesn't waste time dealing with something which isn't real.
- James
--
James Morris
<[email protected]>
On Tue, 2005-04-05 at 01:05 -0400, James Morris wrote:
> Evgeniy,
>
> Please send networking patches to [email protected].
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
On Tue, 2005-04-05 at 01:10 -0400, James Morris wrote:
> On Tue, 5 Apr 2005, James Morris wrote:
>
> > A few questions:
>
> Also, please allow cn_add_callback() allow it to be passed a NULL
> callback function, so the caller doesn't pass in a dummy function and your
> code doesn't waste time dealing with something which isn't real.
Why can anyone want to add callback that will not supposed to be
usefull?
Callback is called when someone sends netlink message with appropriate
idx/val inside, if there is no registered callback with such ID,
nothing will be called and skb will be freed.
>
> - James
--
Evgeniy Polyakov
Crash is better than data corruption -- Arthur Grabowski
On Tue, Apr 05, 2005 at 11:03:16AM +0400, Evgeniy Polyakov wrote:
>
> I received comments and feature requests from Herbert Xu and Jamal Hadi
> Salim,
> almost all were successfully resolved.
Please do not construe my involvement in these threads as endorsement
for this system.
In fact to this day I still don't understand what problems this thing is
meant to solve.
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Tue, 2005-04-05 at 01:10 -0400, Herbert Xu wrote:
>On Tue, Apr 05, 2005 at 11:03:16AM +0400, Evgeniy Polyakov wrote:
>>
>> I received comments and feature requests from Herbert Xu and Jamal Hadi
>> Salim,
>> almost all were successfully resolved.
>
>Please do not construe my involvement in these threads as endorsement
>for this system.
Sure.
I remember you are against it :).
>In fact to this day I still don't understand what problems this thing is
>meant to solve.
Hmm, what else can I add to my words?
May be checking the size of the code needed to broadcast kobject changes
in kobject_uevent.c for example...
Netlink socket allocation + skb handling against call to cn_netlink_send().
>--
>Visit Openswan at http://www.openswan.org/
>Email: Herbert Xu ~{PmV>HI~} <[email protected]>
>Home Page: http://gondor.apana.org.au/~herbert/
>PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
Evgeniy Polyakov
Crash is better than data corruption -- Arthur Grabowski
On Tue, 2005-04-05 at 11:34 +0400, Evgeniy Polyakov wrote:
> On Tue, 2005-04-05 at 01:10 -0400, Herbert Xu wrote:
>
> >In fact to this day I still don't understand what problems this thing is
> >meant to solve.
>
> Hmm, what else can I add to my words?
> May be checking the size of the code needed to broadcast kobject changes
> in kobject_uevent.c for example...
> Netlink socket allocation + skb handling against call to cn_netlink_send().
And It's the same for the fork connector. It allows to send a message to
a user space application when a fork occurs by adding only one line (two
with the #include) in the kernel/fork.c file. Thus, the netlink
connector is a very simple and fast mechanism when you need to send a
small amount of information from kernel space to user space.
Regards,
Guillaume
To be fair to Evgeniy I am not against the Konnector idea. I think that
it is a useful feature to have an easy to use messaging between
kernel-kernel and kernel-userspace. The fact that he leveraged netlink
instead of inventing things is a bonus. Having said that i have not
seriously scrutinized the code - and i think the idea of this new thing
hes tossing around called CBUS maybe pushing it.
cheers,
jamal
On Tue, 2005-04-05 at 03:34, Evgeniy Polyakov wrote:
> On Tue, 2005-04-05 at 01:10 -0400, Herbert Xu wrote:
> >On Tue, Apr 05, 2005 at 11:03:16AM +0400, Evgeniy Polyakov wrote:
> >>
> >> I received comments and feature requests from Herbert Xu and Jamal Hadi
> >> Salim,
> >> almost all were successfully resolved.
> >
> >Please do not construe my involvement in these threads as endorsement
> >for this system.
>
> Sure.
> I remember you are against it :).
>
> >In fact to this day I still don't understand what problems this thing is
> >meant to solve.
>
> Hmm, what else can I add to my words?
> May be checking the size of the code needed to broadcast kobject changes
> in kobject_uevent.c for example...
> Netlink socket allocation + skb handling against call to cn_netlink_send().
>
> >--
> >Visit Openswan at http://www.openswan.org/
> >Email: Herbert Xu ~{PmV>HI~} <[email protected]>
> >Home Page: http://gondor.apana.org.au/~herbert/
> >PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
>
and, oh yeah - wheres the documentation Evgeniy? ;->
cheers,
jamal
On Tue, 2005-04-05 at 06:44, jamal wrote:
> To be fair to Evgeniy I am not against the Konnector idea. I think that
> it is a useful feature to have an easy to use messaging between
> kernel-kernel and kernel-userspace. The fact that he leveraged netlink
> instead of inventing things is a bonus. Having said that i have not
> seriously scrutinized the code - and i think the idea of this new thing
> hes tossing around called CBUS maybe pushing it.
>
> cheers,
> jamal
>
> On Tue, 2005-04-05 at 03:34, Evgeniy Polyakov wrote:
> > On Tue, 2005-04-05 at 01:10 -0400, Herbert Xu wrote:
> > >On Tue, Apr 05, 2005 at 11:03:16AM +0400, Evgeniy Polyakov wrote:
> > >>
> > >> I received comments and feature requests from Herbert Xu and Jamal Hadi
> > >> Salim,
> > >> almost all were successfully resolved.
> > >
> > >Please do not construe my involvement in these threads as endorsement
> > >for this system.
> >
> > Sure.
> > I remember you are against it :).
> >
> > >In fact to this day I still don't understand what problems this thing is
> > >meant to solve.
> >
> > Hmm, what else can I add to my words?
> > May be checking the size of the code needed to broadcast kobject changes
> > in kobject_uevent.c for example...
> > Netlink socket allocation + skb handling against call to cn_netlink_send().
> >
> > >--
> > >Visit Openswan at http://www.openswan.org/
> > >Email: Herbert Xu ~{PmV>HI~} <[email protected]>
> > >Home Page: http://gondor.apana.org.au/~herbert/
> > >PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> >
>
>
>
On Tue, 2005-04-05 at 07:00 -0400, jamal wrote:
> and, oh yeah - wheres the documentation Evgeniy? ;->
In the tree :)
Documentation/connector/connector.txt - some notes, API.
Documentation/connector/cn_test.c - kernel example.
Uses cn_netlink_send(), notification feature.
I will send today a pathc that adds in-source documentation
bits with some code cleanups.
> cheers,
> jamal
>
> On Tue, 2005-04-05 at 06:44, jamal wrote:
> > To be fair to Evgeniy I am not against the Konnector idea. I think that
> > it is a useful feature to have an easy to use messaging between
> > kernel-kernel and kernel-userspace. The fact that he leveraged netlink
> > instead of inventing things is a bonus. Having said that i have not
> > seriously scrutinized the code - and i think the idea of this new thing
> > hes tossing around called CBUS maybe pushing it.
> >
> > cheers,
> > jamal
> >
> > On Tue, 2005-04-05 at 03:34, Evgeniy Polyakov wrote:
> > > On Tue, 2005-04-05 at 01:10 -0400, Herbert Xu wrote:
> > > >On Tue, Apr 05, 2005 at 11:03:16AM +0400, Evgeniy Polyakov wrote:
> > > >>
> > > >> I received comments and feature requests from Herbert Xu and Jamal Hadi
> > > >> Salim,
> > > >> almost all were successfully resolved.
> > > >
> > > >Please do not construe my involvement in these threads as endorsement
> > > >for this system.
> > >
> > > Sure.
> > > I remember you are against it :).
> > >
> > > >In fact to this day I still don't understand what problems this thing is
> > > >meant to solve.
> > >
> > > Hmm, what else can I add to my words?
> > > May be checking the size of the code needed to broadcast kobject changes
> > > in kobject_uevent.c for example...
> > > Netlink socket allocation + skb handling against call to cn_netlink_send().
> > >
> > > >--
> > > >Visit Openswan at http://www.openswan.org/
> > > >Email: Herbert Xu ~{PmV>HI~} <[email protected]>
> > > >Home Page: http://gondor.apana.org.au/~herbert/
> > > >PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> > >
> >
> >
> >
--
Evgeniy Polyakov
Crash is better than data corruption -- Arthur Grabowski
On Tue, 5 Apr 2005, Evgeniy Polyakov wrote:
> On Tue, 2005-04-05 at 01:05 -0400, James Morris wrote:
> > Evgeniy,
> >
> > Please send networking patches to [email protected].
>
> It was sent there two times.
I googled around quite a lot to track the origin of the patches down but
didn't do the obvious and look in the netdev archives, sorry.
- James
--
James Morris
<[email protected]>