From: Max Krasnyansky <maxk@qualcomm.com>
To: "David S. Miller" <davem@redhat.com>
Cc: acme@conectiva.com.br, linux-kernel@vger.kernel.org, netdev@oss.sgi.com
Subject: Re: [BK ChangeSet@1.1118.1.1] new module infrastructure for net_proto_family
Date: Wed, 23 Apr 2003 18:41:56 -0700 [thread overview]
Message-ID: <5.1.0.14.2.20030423182014.07ec6140@unixmail.qualcomm.com> (raw)
In-Reply-To: <20030423.163043.41633133.davem@redhat.com>
At 04:30 PM 4/23/2003, David S. Miller wrote:
> From: Max Krasnyansky <maxk@qualcomm.com>
> Date: Wed, 23 Apr 2003 15:51:11 -0700
>
> >This is just the first part, DaveM already merged the second part,
> >that deals with struct sock
>
> That's exactly what surprised me. He rejected complete patch and
> accepted something incomplete and broken.
>
>No, it was not broken, because he told me completely where he
>was going with his changes.
Of course it was and still is. New socket is allocated without incrementing
modules ref count in sys_accept().
>He was building infrastructure piece by piece, and that's always an acceptable
>way to do things as long as it is explained where one is going with the changes.
Oh, I see. And I just sent a patch without any explanation. Ok.
(you might want to reread our original discussion again).
>Your stuff was unacceptable from the start because you didn't put
>the ->owner into the protocol ops.
But you didn't tell me that. You just said that it's "an ugly hack" without
giving any other feedback.
->owner field in protocol ops did come up during discussion (I think Rusty brought
that up) and I explained why it shouldn't be there. But again there was no feed back
from you. You just ignored that thread at some point.
btw I still don't see ->owner in protocol ops. I read archives of netdev. You guys
didn't even talk about that.
Anyway it's not important who said what now. You chose to ignore stuff that I did, fine.
What about this though
>>struct sock *sk_alloc(int family, int priority, int zero_it, kmem_cache_t *slab)
>>{
>>- struct sock *sk;
>>-
>>+ struct sock *sk = NULL;
>>+
>>+ if (!net_family_get(family))
>>+ goto out;
>Ok. This is wrong. Which should be clear from reading the thread that I mentioned.
>Owner of the net_proto_family is not necessarily the owner of the 'struct sock'.
>Example: af_inet module registers net_proto_family but udp module owns the socket.
>(not that IPv4 code is modular but just an example). Another example would be Bluetooth.
>We have Bluetooth core that registers Bluetooth proto_family and several modules
>that handle specific protocols (l2cap, rfcomm, etc).
>
>Also net_proto_family has pretty much nothing to do with the struct sock. The only reason
>we would want to hold reference to the module is if the module has replaced default
>callbacks (i.e. sk->data_ready(), etc).
>So my point is we need sk->owner field.
>
>I'd also prefer to see sock->owner which gives more flexibility (i.e. net_proto_family can
>be unregistered while struct socket still exist, etc).
>net_family_get/put() makes no sense net_proto_family has only one function npf->create()
>which is referenced only from net/socket.c. struct socket should inherit owner field from
>struct net_proto_family by default but protocol should be able to assign ownership to a
>different module if it needs to.
Max
next prev parent reply other threads:[~2003-04-24 1:30 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2003-04-23 19:13 [BK ChangeSet@1.1118.1.1] new module infrastructure for net_proto_family Max Krasnyansky
2003-04-23 19:26 ` Arnaldo Carvalho de Melo
2003-04-23 22:51 ` Max Krasnyansky
2003-04-23 23:30 ` David S. Miller
2003-04-24 1:41 ` Max Krasnyansky [this message]
2003-04-24 3:29 ` David S. Miller
2003-04-24 16:43 ` Max Krasnyansky
2003-04-24 6:44 ` Arnaldo Carvalho de Melo
2003-04-24 19:33 ` Max Krasnyansky
2003-04-24 23:02 ` Arnaldo Carvalho de Melo
2003-04-25 0:40 ` Max Krasnyansky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5.1.0.14.2.20030423182014.07ec6140@unixmail.qualcomm.com \
--to=maxk@qualcomm.com \
--cc=acme@conectiva.com.br \
--cc=davem@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@oss.sgi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).