| Message ID | 20230322223329.48949-1-eric_sage@apple.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | netfilter: nfnetlink_queue: enable classid socket info retrieval | expand |
eric_sage@apple.com <eric_sage@apple.com> wrote: > From: Eric Sage <eric_sage@apple.com> > > This enables associating a socket with a v1 net_cls cgroup. Useful for > applying a per-cgroup policy when processing packets in userspace. > > Signed-off-by: Eric Sage <eric_sage@apple.com> > --- > .../uapi/linux/netfilter/nfnetlink_queue.h | 4 ++- > net/netfilter/nfnetlink_queue.c | 27 +++++++++++++++++++ > 2 files changed, 30 insertions(+), 1 deletion(-) > > diff --git a/include/uapi/linux/netfilter/nfnetlink_queue.h b/include/uapi/linux/netfilter/nfnetlink_queue.h > index ef7c97f21a15..9fbc8c49bd6d 100644 > --- a/include/uapi/linux/netfilter/nfnetlink_queue.h > +++ b/include/uapi/linux/netfilter/nfnetlink_queue.h > @@ -62,6 +62,7 @@ enum nfqnl_attr_type { > NFQA_VLAN, /* nested attribute: packet vlan info */ > NFQA_L2HDR, /* full L2 header */ > NFQA_PRIORITY, /* skb->priority */ > + NFQA_CLASSID, /* __u32 cgroup classid */ > > __NFQA_MAX > }; > @@ -116,7 +117,8 @@ enum nfqnl_attr_config { > #define NFQA_CFG_F_GSO (1 << 2) > #define NFQA_CFG_F_UID_GID (1 << 3) > #define NFQA_CFG_F_SECCTX (1 << 4) > -#define NFQA_CFG_F_MAX (1 << 5) > +#define NFQA_CFG_F_CLASSID (1 << 5) > +#define NFQA_CFG_F_MAX (1 << 6) > > /* flags for NFQA_SKB_INFO */ > /* packet appears to have wrong checksums, but they are ok */ > diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c > index 87a9009d5234..8c513a2e0e30 100644 > --- a/net/netfilter/nfnetlink_queue.c > +++ b/net/netfilter/nfnetlink_queue.c > @@ -301,6 +301,25 @@ static int nfqnl_put_sk_uidgid(struct sk_buff *skb, struct sock *sk) > return -1; > } > > +static int nfqnl_put_sk_classid(struct sk_buff *skb, struct sock *sk) > +{ > + u32 classid; > + > + if (!sk_fullsock(sk)) > + return 0; > + > + read_lock_bh(sk->sk_callback_lock); I don't think there is a need for this lock here. > + if (nla_put_be32(skb, NFQA_CLASSID, htonl(classid))) > + goto nla_put_failure; > + read_unlock_bh(sk->sk_callback_lock); I think this can be something like: static int nfqnl_put_sk_classid(struct sk_buff *skb, const struct sock *sk) { #if CONFIG_CGROUP_NET_CLASSID if (sk && sk_fullsock(sk)) { u32 classid = htonl(sock_cgroup_classid(&sk->sk_cgrp_data); if (classid && nla_put_be32(skb, NFQA_CLASSID, htonl(classid))) return -1; } #endif return 0; } > + if (queue->flags & NFQA_CFG_F_CLASSID) { > + size += nla_total_size(sizeof(u_int32_t)); /* classid */ > + } Not sure its worth adding a new queue flag for this. Uid and gid is a bit of extra work but I'd guess that the sk deref isn't that noticeable compared to the nfnetlink cost. So probably just include the size unconditionally in the initial calculation and then just: if (nfqnl_put_sk_classid(skb, entskb->sk) < 0) goto nla_put_failure; What do you think? Other than that this seems fine.
Hi,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on horms-ipvs/master]
[also build test ERROR on linus/master v6.3-rc3 next-20230322]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/eric_sage-apple-com/netfilter-nfnetlink_queue-enable-classid-socket-info-retrieval/20230323-073405
base: https://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs.git master
patch link: https://lore.kernel.org/r/20230322223329.48949-1-eric_sage%40apple.com
patch subject: [PATCH] netfilter: nfnetlink_queue: enable classid socket info retrieval
config: i386-randconfig-a005 (https://download.01.org/0day-ci/archive/20230323/202303231155.PNtozlaS-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/c28410a18b276a059551ce4ffad5d3721ea8cbf5
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review eric_sage-apple-com/netfilter-nfnetlink_queue-enable-classid-socket-info-retrieval/20230323-073405
git checkout c28410a18b276a059551ce4ffad5d3721ea8cbf5
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 olddefconfig
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash net/netfilter/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303231155.PNtozlaS-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/linux/spinlock.h:304,
from include/linux/kref.h:16,
from include/linux/mm_types.h:8,
from include/linux/buildid.h:5,
from include/linux/module.h:14,
from net/netfilter/nfnetlink_queue.c:16:
net/netfilter/nfnetlink_queue.c: In function 'nfqnl_put_sk_classid':
>> net/netfilter/nfnetlink_queue.c:311:24: error: incompatible type for argument 1 of '_raw_read_lock_bh'
311 | read_lock_bh(sk->sk_callback_lock);
| ~~^~~~~~~~~~~~~~~~~~
| |
| rwlock_t
include/linux/rwlock.h:93:59: note: in definition of macro 'read_lock_bh'
93 | #define read_lock_bh(lock) _raw_read_lock_bh(lock)
| ^~~~
In file included from include/linux/spinlock_api_smp.h:183,
from include/linux/spinlock.h:311,
from include/linux/kref.h:16,
from include/linux/mm_types.h:8,
from include/linux/buildid.h:5,
from include/linux/module.h:14,
from net/netfilter/nfnetlink_queue.c:16:
include/linux/rwlock_api_smp.h:21:45: note: expected 'rwlock_t *' but argument is of type 'rwlock_t'
21 | void __lockfunc _raw_read_lock_bh(rwlock_t *lock) __acquires(lock);
| ~~~~~~~~~~^~~~
>> net/netfilter/nfnetlink_queue.c:312:30: error: 'entskb' undeclared (first use in this function)
312 | sock_cgroup_classid(&entskb->sk->sk_cgrp_data);
| ^~~~~~
net/netfilter/nfnetlink_queue.c:312:30: note: each undeclared identifier is reported only once for each function it appears in
In file included from include/linux/spinlock.h:304,
from include/linux/kref.h:16,
from include/linux/mm_types.h:8,
from include/linux/buildid.h:5,
from include/linux/module.h:14,
from net/netfilter/nfnetlink_queue.c:16:
>> net/netfilter/nfnetlink_queue.c:315:26: error: incompatible type for argument 1 of '_raw_read_unlock_bh'
315 | read_unlock_bh(sk->sk_callback_lock);
| ~~^~~~~~~~~~~~~~~~~~
| |
| rwlock_t
include/linux/rwlock.h:106:61: note: in definition of macro 'read_unlock_bh'
106 | #define read_unlock_bh(lock) _raw_read_unlock_bh(lock)
| ^~~~
In file included from include/linux/spinlock_api_smp.h:183,
from include/linux/spinlock.h:311,
from include/linux/kref.h:16,
from include/linux/mm_types.h:8,
from include/linux/buildid.h:5,
from include/linux/module.h:14,
from net/netfilter/nfnetlink_queue.c:16:
include/linux/rwlock_api_smp.h:33:47: note: expected 'rwlock_t *' but argument is of type 'rwlock_t'
33 | void __lockfunc _raw_read_unlock_bh(rwlock_t *lock) __releases(lock);
| ~~~~~~~~~~^~~~
In file included from include/linux/spinlock.h:304,
from include/linux/kref.h:16,
from include/linux/mm_types.h:8,
from include/linux/buildid.h:5,
from include/linux/module.h:14,
from net/netfilter/nfnetlink_queue.c:16:
net/netfilter/nfnetlink_queue.c:319:26: error: incompatible type for argument 1 of '_raw_read_unlock_bh'
319 | read_unlock_bh(sk->sk_callback_lock);
| ~~^~~~~~~~~~~~~~~~~~
| |
| rwlock_t
include/linux/rwlock.h:106:61: note: in definition of macro 'read_unlock_bh'
106 | #define read_unlock_bh(lock) _raw_read_unlock_bh(lock)
| ^~~~
In file included from include/linux/spinlock_api_smp.h:183,
from include/linux/spinlock.h:311,
from include/linux/kref.h:16,
from include/linux/mm_types.h:8,
from include/linux/buildid.h:5,
from include/linux/module.h:14,
from net/netfilter/nfnetlink_queue.c:16:
include/linux/rwlock_api_smp.h:33:47: note: expected 'rwlock_t *' but argument is of type 'rwlock_t'
33 | void __lockfunc _raw_read_unlock_bh(rwlock_t *lock) __releases(lock);
| ~~~~~~~~~~^~~~
vim +/_raw_read_lock_bh +311 net/netfilter/nfnetlink_queue.c
303
304 static int nfqnl_put_sk_classid(struct sk_buff *skb, struct sock *sk)
305 {
306 u32 classid;
307
308 if (!sk_fullsock(sk))
309 return 0;
310
> 311 read_lock_bh(sk->sk_callback_lock);
> 312 sock_cgroup_classid(&entskb->sk->sk_cgrp_data);
313 if (nla_put_be32(skb, NFQA_CLASSID, htonl(classid)))
314 goto nla_put_failure;
> 315 read_unlock_bh(sk->sk_callback_lock);
316 return 0;
317
318 nla_put_failure:
319 read_unlock_bh(sk->sk_callback_lock);
320 return -1;
321 }
322
Hi,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on horms-ipvs/master]
[also build test ERROR on linus/master v6.3-rc3 next-20230322]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/eric_sage-apple-com/netfilter-nfnetlink_queue-enable-classid-socket-info-retrieval/20230323-073405
base: https://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs.git master
patch link: https://lore.kernel.org/r/20230322223329.48949-1-eric_sage%40apple.com
patch subject: [PATCH] netfilter: nfnetlink_queue: enable classid socket info retrieval
config: m68k-defconfig (https://download.01.org/0day-ci/archive/20230323/202303231242.QaYElp9P-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/c28410a18b276a059551ce4ffad5d3721ea8cbf5
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review eric_sage-apple-com/netfilter-nfnetlink_queue-enable-classid-socket-info-retrieval/20230323-073405
git checkout c28410a18b276a059551ce4ffad5d3721ea8cbf5
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash net/netfilter/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303231242.QaYElp9P-lkp@intel.com/
All errors (new ones prefixed by >>):
net/netfilter/nfnetlink_queue.c: In function 'nfqnl_put_sk_classid':
>> net/netfilter/nfnetlink_queue.c:312:9: error: implicit declaration of function 'sock_cgroup_classid' [-Werror=implicit-function-declaration]
312 | sock_cgroup_classid(&entskb->sk->sk_cgrp_data);
| ^~~~~~~~~~~~~~~~~~~
net/netfilter/nfnetlink_queue.c:312:30: error: 'entskb' undeclared (first use in this function)
312 | sock_cgroup_classid(&entskb->sk->sk_cgrp_data);
| ^~~~~~
net/netfilter/nfnetlink_queue.c:312:30: note: each undeclared identifier is reported only once for each function it appears in
cc1: some warnings being treated as errors
vim +/sock_cgroup_classid +312 net/netfilter/nfnetlink_queue.c
303
304 static int nfqnl_put_sk_classid(struct sk_buff *skb, struct sock *sk)
305 {
306 u32 classid;
307
308 if (!sk_fullsock(sk))
309 return 0;
310
311 read_lock_bh(sk->sk_callback_lock);
> 312 sock_cgroup_classid(&entskb->sk->sk_cgrp_data);
313 if (nla_put_be32(skb, NFQA_CLASSID, htonl(classid)))
314 goto nla_put_failure;
315 read_unlock_bh(sk->sk_callback_lock);
316 return 0;
317
318 nla_put_failure:
319 read_unlock_bh(sk->sk_callback_lock);
320 return -1;
321 }
322
diff --git a/include/uapi/linux/netfilter/nfnetlink_queue.h b/include/uapi/linux/netfilter/nfnetlink_queue.h index ef7c97f21a15..9fbc8c49bd6d 100644 --- a/include/uapi/linux/netfilter/nfnetlink_queue.h +++ b/include/uapi/linux/netfilter/nfnetlink_queue.h @@ -62,6 +62,7 @@ enum nfqnl_attr_type { NFQA_VLAN, /* nested attribute: packet vlan info */ NFQA_L2HDR, /* full L2 header */ NFQA_PRIORITY, /* skb->priority */ + NFQA_CLASSID, /* __u32 cgroup classid */ __NFQA_MAX }; @@ -116,7 +117,8 @@ enum nfqnl_attr_config { #define NFQA_CFG_F_GSO (1 << 2) #define NFQA_CFG_F_UID_GID (1 << 3) #define NFQA_CFG_F_SECCTX (1 << 4) -#define NFQA_CFG_F_MAX (1 << 5) +#define NFQA_CFG_F_CLASSID (1 << 5) +#define NFQA_CFG_F_MAX (1 << 6) /* flags for NFQA_SKB_INFO */ /* packet appears to have wrong checksums, but they are ok */ diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c index 87a9009d5234..8c513a2e0e30 100644 --- a/net/netfilter/nfnetlink_queue.c +++ b/net/netfilter/nfnetlink_queue.c @@ -301,6 +301,25 @@ static int nfqnl_put_sk_uidgid(struct sk_buff *skb, struct sock *sk) return -1; } +static int nfqnl_put_sk_classid(struct sk_buff *skb, struct sock *sk) +{ + u32 classid; + + if (!sk_fullsock(sk)) + return 0; + + read_lock_bh(sk->sk_callback_lock); + sock_cgroup_classid(&entskb->sk->sk_cgrp_data); + if (nla_put_be32(skb, NFQA_CLASSID, htonl(classid))) + goto nla_put_failure; + read_unlock_bh(sk->sk_callback_lock); + return 0; + +nla_put_failure: + read_unlock_bh(sk->sk_callback_lock); + return -1; +} + static u32 nfqnl_get_sk_secctx(struct sk_buff *skb, char **secdata) { u32 seclen = 0; @@ -461,6 +480,10 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue, + nla_total_size(sizeof(u_int32_t))); /* gid */ } + if (queue->flags & NFQA_CFG_F_CLASSID) { + size += nla_total_size(sizeof(u_int32_t)); /* classid */ + } + if ((queue->flags & NFQA_CFG_F_SECCTX) && entskb->sk) { seclen = nfqnl_get_sk_secctx(entskb, &secdata); if (seclen) @@ -599,6 +622,10 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue, nfqnl_put_sk_uidgid(skb, entskb->sk) < 0) goto nla_put_failure; + if ((queue->flags & NFQA_CFG_F_CLASSID) && entskb->sk && + nfqnl_put_sk_classid(skb, entskb->sk) < 0) + goto nla_put_failure; + if (seclen && nla_put(skb, NFQA_SECCTX, seclen, secdata)) goto nla_put_failure;