Thread (13 messages) 13 messages, 4 authors, 2003-05-01

Re: [PATCH] af_pppox: create module infrastructure for protocol modules

From: Arnaldo Carvalho de Melo <hidden>
Date: 2003-04-29 22:07:46

Em Tue, Apr 29, 2003 at 01:05:08PM -0700, Max Krasnyansky escreveu:
At 11:54 PM 4/28/2003, Arnaldo Carvalho de Melo wrote:
quoted
Em Mon, Apr 28, 2003 at 10:27:28PM -0700, David S. Miller escreveu:
quoted
   From: Arnaldo Carvalho de Melo [off-list ref]
   Date: Tue, 29 Apr 2003 03:12:27 -0300
quoted
quoted
Max, take a look and see if this same approach can be used in bluetooth, I
bet it can, its just a matter of not using struct net_proto_family for
bt_proto, just like pppox already was doing before my changes :-)
quoted
quoted
Something similar can be done for ipv4/ipv6 by adding a struct module
*owner member to struct inet_protosw etc. etc.
quoted
yes
quoted
quoted
Although the idea is conceptually sound, you miss one crucial thing.
Such struct sock's reference _TWO_ modules, the "PPPOE" module
and the "PPPOX" module.
This is not a problem. PPPOX (or ipv4/ipv6) module is not going to 
go away simply because PPPOE (or TCP/UDP) uses symbols from it.
There is no need to bump refcounters here. 
I think it's safe to say that protocols within the family always use 
symbols from main family module (they have to at list register/unregister).
So this is naturally taken care of.
Because at the core level we don't know (and perhaps don't want to know) what
the specific net family module nor its protocol modules are doing
 
quoted
But what is the problem? at pppox_sk_alloc time I bump the PPPOE module refcnt,
making it safe, then it calls sk_alloc where it bumps the PPPOX module, making
it safe as well, so I'm taking care of both PPPOE and PPPOX.
Bumping refcount for PPPOX is an overhead without any benefits. Read above.
 
quoted
quoted
      struct module   *owner;
quoted
This one is the net_families[net_family]->owner
quoted
quoted
      struct module   *sub_owner;
quoted
this one is the pppox_protos[protocol]->owner
quoted
I thought about it, but I don't see why the current scheme doesn't handle
it, care to elaborate a bit more? I don't doubt that I may be missing some
subtlety :-)
Ok. I'm going to ask this here again (to make sure that it's answered :))
Hey, I answered already, but it seems you disagree with my view 8)
 
- Why do we have to bump module refcount for 'struct sock' with _default_ callbacks ?
My answer is that we don't have to. I'd even say that we shouldn't. Module is not 
referenced from 'struct sock' in that case.
We just don't assume nothing about the net protocol family implementation, but now
that the infrastructure is in place we can surely study further optimizations that
don't break its layering abstraction.
 
- Why are we not allowing net_family unregistration until all family sockets
are gone?  From what I see it's only because current code does
'module_put(net_families[family]->owner)'. But we don't care whether
net_family is registered or not if we know who owns the socket (i.e.
sock->owner).
but we introduce aditional overhead in all struct sock created, why it is so important
to have the net_family unregistration done early rather than after all the struct sock
are freed?
 
So far nobody gave me a clear answer to those questions. If we don't have to
meet those two restriction I don't see the point in creating all this
net_family_get()/xxx_family_get(), etc, infrastructure. Patches and BK stuff
that I've seen so far added more code (and a bit more overhead) than my
original patch with sock->owner/sk->owner/sk_set_owner().  Yet they provide
no additional flexibility or functionality.
See, its a tradeoff, we don't bloat struct sock paying a bit of overhead in
other area.
 
I hate to repeat myself and please forgive me for that. But how is the
current infrastructure better than following ?
See my views above. And I'd love, as you, to have more people discussing this,
but most people I think that could help with this are damn busy with other
work, so at least we have a solid infrastructure in place and can revisit this
later if there is interest in further enhancing this.

- Arnaldo
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help