mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Anjali Kulkarni <anjali.k.kulkarni@oracle.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: "davem@davemloft.net" <davem@davemloft.net>,
	"edumazet@google.com" <edumazet@google.com>,
	"pabeni@redhat.com" <pabeni@redhat.com>,
	"zbr@ioremap.net" <zbr@ioremap.net>,
	"brauner@kernel.org" <brauner@kernel.org>,
	"johannes@sipsolutions.net" <johannes@sipsolutions.net>,
	"ecree.xilinx@gmail.com" <ecree.xilinx@gmail.com>,
	"leon@kernel.org" <leon@kernel.org>,
	"keescook@chromium.org" <keescook@chromium.org>,
	"socketcan@hartkopp.net" <socketcan@hartkopp.net>,
	"petrm@nvidia.com" <petrm@nvidia.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: [PATCH v1 2/5] connector/cn_proc: Add filtering to fix some bugs
Date: Wed, 15 Mar 2023 20:12:01 +0000	[thread overview]
Message-ID: <D1BAB522-682D-436D-B542-C4444918189F@oracle.com> (raw)
In-Reply-To: <20230315125004.5b203529@kernel.org>


>> 
>> The unbind call will not work because it is for the case of adding and deleting group memberships and hence called from netlink_setsockopt() when  NETLINK_DROP_MEMBERSHIP option is given. We would not be able to distinguish between the drop membership & release cases.
>> The notifier call seems to be for blocked clients? Am not sure if the we need to block/wait on this call to be notified to free/release. Also, the API does not pass in struct sock to free what we want, so we will need to change that everywhere it is currently used.
> 
> I think that adding the new release callback is acceptable.
> I haven't seen your v2 before replying.

Thanks, so I will wait for your comments on v2 before sending updated patch.

> 
>> As for using sk_user_data - this field seems to be used by different applications in different ways depending on how they need to use data. If we use a new field in netlink_sock, we would need to add a new API function to allocate this member, similar to release, because it seems you cannot access netlink_sock outside of af_netlink, or at least I do not see any current access to it, and functions like nlk_sk are static. Also, if we add an allocation function, we won’t know the first time the client sends it’s data (we need to know “initial” in the patches), so we will need to add a new field in the socket to indicate first access or add a lot more infrastructure in cn_proc to store each client’s information.
> 
> Alright, I guess we can risk the sk_user_data, and see if anything
> explodes. Normally higher protocol wraps the socket structure in its
> own structure and the sk_user_data pointer is for an in-kernel user
> of the socket (i.e. in kernel client). But "classic netlink" is all
> legacy code (new subsystem use the generic netlink overlay) so trying
> to decode now why things are the way they are and fixing the structure
> would be a major effort :(

Ok, thanks!


  reply	other threads:[~2023-03-15 20:12 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-10 22:15 [PATCH v1 0/5] Process connector bug fixes & enhancements Anjali Kulkarni
2023-03-10 22:15 ` [PATCH v1 1/5] netlink: Reverse the patch which removed filtering Anjali Kulkarni
2023-03-10 22:15 ` [PATCH v1 2/5] connector/cn_proc: Add filtering to fix some bugs Anjali Kulkarni
2023-03-14  0:24   ` Jakub Kicinski
2023-03-14  2:32     ` Anjali Kulkarni
2023-03-15  4:59       ` Jakub Kicinski
2023-03-15 19:08         ` Anjali Kulkarni
2023-03-15 19:13           ` Anjali Kulkarni
2023-03-15 19:50           ` Jakub Kicinski
2023-03-15 20:12             ` Anjali Kulkarni [this message]
2023-03-14  8:38     ` Christian Brauner
2023-03-14 18:51       ` Anjali Kulkarni
2023-04-01 18:32       ` Anjali Kulkarni
2023-03-10 22:15 ` [PATCH v1 3/5] connector/cn_proc: Test code for proc connector Anjali Kulkarni
2023-03-10 22:15 ` [PATCH v1 4/5] connector/cn_proc: Allow non-root users access Anjali Kulkarni
2023-03-10 22:15 ` [PATCH v1 5/5] connector/cn_proc: Performance improvements Anjali Kulkarni

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=D1BAB522-682D-436D-B542-C4444918189F@oracle.com \
    --to=anjali.k.kulkarni@oracle.com \
    --cc=brauner@kernel.org \
    --cc=davem@davemloft.net \
    --cc=ecree.xilinx@gmail.com \
    --cc=edumazet@google.com \
    --cc=johannes@sipsolutions.net \
    --cc=keescook@chromium.org \
    --cc=kuba@kernel.org \
    --cc=leon@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=petrm@nvidia.com \
    --cc=socketcan@hartkopp.net \
    --cc=zbr@ioremap.net \
    /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

all inboxes | Powered by JetHome®