Re: [nft PATCH RFC] monitor: Support printing processes which caused the event

[Florian Westphal <fw@strlen.de>]

Phil Sutter <phil@nwl.cc> wrote:
> Hi,
> 
> On Wed, May 10, 2017 at 01:57:48PM +0200, Florian Westphal wrote:
> > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > On Wed, May 10, 2017 at 01:27:24PM +0200, Florian Westphal wrote:
> > > > While most programs set it to their process id there is no guarantee.
> > > > Its just a (unique) 32 bit identifier.
> > > 
> > > It's actually the kernel that decides what portID the socket gets
> > > IIRC. For the first socket it uses the process ID, then for follow up
> > > sockets, it just looks for a spare ID in the negative range of the 32
> > > bit, if my memory serves well.
> > 
> > Argh, yes, of course...
> > 
> > > > Afaics one has to use /proc/net/netlink to map the portid to the inode
> > > > and then walk /proc/*/fd/* to find the socket with that inode.
> > > > 
> > > > Perhaps there is a simpler way, maybe you can check what ss is doing
> > > > and what info can be obtained via netlink diag.
> > > 
> > > I wouldn't be surprise if we need more kernel infrastructure to deal
> > > with this. Parsing /proc for a netlink thing is definitely not ideal.
> > 
> > Yes.  From nft monitor point of view the most easy solution would be
> > if the process id (or even the name?) would be sent back to userspace in a netlink
> > attribute.  Do you think we can extend nf_tables to include
> > get_task_comm() name and/or pid when/if we send update notifications?
> > 
> > (The pid is actually not that useful as process might have exited
> > already).
> 
> The question is where to put it. Looking at the netlink message
> structure, I see two options:
> 
> A) Extend struct nfgenmsg to contain PID and process name (a buffer of
>    length TASK_COMM_LEN).
> B) Add type-specific attributes for each type, like NFTA_RULE_PID and
>    NFTA_RULE_PNAME.
> 
> The problem with A) is that it will break older user space expecting
> sizeof(struct nfgenmsg) to be shorter.

Right, A) is a non-starter.

> Additional attributes should not
> be a problem here, but having to add them for each object type seems to
> be a really ugly solution.

I don't find it ugly, but alternatively we could add a new type of info
sent at the beginning of the commit phase (before all the table/rule etc
updates) and include it there.