* Re: [PATCH nf-next] netfilter: ctnetlink: remove get_ct indirection
2021-01-19 9:21 [PATCH nf-next] netfilter: ctnetlink: remove get_ct indirection Florian Westphal
@ 2021-01-20 14:37 ` kernel test robot
2021-01-20 14:58 ` kernel test robot
1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2021-01-20 14:37 UTC (permalink / raw)
To: Florian Westphal, netfilter-devel; +Cc: kbuild-all, Florian Westphal
[-- Attachment #1: Type: text/plain, Size: 6940 bytes --]
Hi Florian,
I love your patch! Perhaps something to improve:
[auto build test WARNING on nf-next/master]
url: https://github.com/0day-ci/linux/commits/Florian-Westphal/netfilter-ctnetlink-remove-get_ct-indirection/20210120-190814
base: https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master
config: nios2-randconfig-r002-20210120 (attached as .config)
compiler: nios2-linux-gcc (GCC) 9.3.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/0day-ci/linux/commit/20f5f1a1d8f0d775b9982e1404cf44840dd0a86a
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Florian-Westphal/netfilter-ctnetlink-remove-get_ct-indirection/20210120-190814
git checkout 20f5f1a1d8f0d775b9982e1404cf44840dd0a86a
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=nios2
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
net/netfilter/nfnetlink_log.c: In function 'nfulnl_log_packet':
net/netfilter/nfnetlink_log.c:743:9: error: implicit declaration of function 'nf_ct_get' [-Werror=implicit-function-declaration]
743 | ct = nf_ct_get(skb, &ctinfo);
| ^~~~~~~~~
>> net/netfilter/nfnetlink_log.c:743:7: warning: assignment to 'struct nf_conn *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
743 | ct = nf_ct_get(skb, &ctinfo);
| ^
cc1: some warnings being treated as errors
vim +743 net/netfilter/nfnetlink_log.c
674
675 /* log handler for internal netfilter logging api */
676 static void
677 nfulnl_log_packet(struct net *net,
678 u_int8_t pf,
679 unsigned int hooknum,
680 const struct sk_buff *skb,
681 const struct net_device *in,
682 const struct net_device *out,
683 const struct nf_loginfo *li_user,
684 const char *prefix)
685 {
686 size_t size;
687 unsigned int data_len;
688 struct nfulnl_instance *inst;
689 const struct nf_loginfo *li;
690 unsigned int qthreshold;
691 unsigned int plen = 0;
692 struct nfnl_log_net *log = nfnl_log_pernet(net);
693 const struct nfnl_ct_hook *nfnl_ct = NULL;
694 struct nf_conn *ct = NULL;
695 enum ip_conntrack_info ctinfo;
696
697 if (li_user && li_user->type == NF_LOG_TYPE_ULOG)
698 li = li_user;
699 else
700 li = &default_loginfo;
701
702 inst = instance_lookup_get(log, li->u.ulog.group);
703 if (!inst)
704 return;
705
706 if (prefix)
707 plen = strlen(prefix) + 1;
708
709 /* FIXME: do we want to make the size calculation conditional based on
710 * what is actually present? way more branches and checks, but more
711 * memory efficient... */
712 size = nlmsg_total_size(sizeof(struct nfgenmsg))
713 + nla_total_size(sizeof(struct nfulnl_msg_packet_hdr))
714 + nla_total_size(sizeof(u_int32_t)) /* ifindex */
715 + nla_total_size(sizeof(u_int32_t)) /* ifindex */
716 #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
717 + nla_total_size(sizeof(u_int32_t)) /* ifindex */
718 + nla_total_size(sizeof(u_int32_t)) /* ifindex */
719 #endif
720 + nla_total_size(sizeof(u_int32_t)) /* mark */
721 + nla_total_size(sizeof(u_int32_t)) /* uid */
722 + nla_total_size(sizeof(u_int32_t)) /* gid */
723 + nla_total_size(plen) /* prefix */
724 + nla_total_size(sizeof(struct nfulnl_msg_packet_hw))
725 + nla_total_size(sizeof(struct nfulnl_msg_packet_timestamp))
726 + nla_total_size(sizeof(struct nfgenmsg)); /* NLMSG_DONE */
727
728 if (in && skb_mac_header_was_set(skb)) {
729 size += nla_total_size(skb->dev->hard_header_len)
730 + nla_total_size(sizeof(u_int16_t)) /* hwtype */
731 + nla_total_size(sizeof(u_int16_t)); /* hwlen */
732 }
733
734 spin_lock_bh(&inst->lock);
735
736 if (inst->flags & NFULNL_CFG_F_SEQ)
737 size += nla_total_size(sizeof(u_int32_t));
738 if (inst->flags & NFULNL_CFG_F_SEQ_GLOBAL)
739 size += nla_total_size(sizeof(u_int32_t));
740 if (inst->flags & NFULNL_CFG_F_CONNTRACK) {
741 nfnl_ct = rcu_dereference(nfnl_ct_hook);
742 if (nfnl_ct != NULL) {
> 743 ct = nf_ct_get(skb, &ctinfo);
744 if (ct != NULL)
745 size += nfnl_ct->build_size(ct);
746 }
747 }
748 if (pf == NFPROTO_NETDEV || pf == NFPROTO_BRIDGE)
749 size += nfulnl_get_bridge_size(skb);
750
751 qthreshold = inst->qthreshold;
752 /* per-rule qthreshold overrides per-instance */
753 if (li->u.ulog.qthreshold)
754 if (qthreshold > li->u.ulog.qthreshold)
755 qthreshold = li->u.ulog.qthreshold;
756
757
758 switch (inst->copy_mode) {
759 case NFULNL_COPY_META:
760 case NFULNL_COPY_NONE:
761 data_len = 0;
762 break;
763
764 case NFULNL_COPY_PACKET:
765 data_len = inst->copy_range;
766 if ((li->u.ulog.flags & NF_LOG_F_COPY_LEN) &&
767 (li->u.ulog.copy_len < data_len))
768 data_len = li->u.ulog.copy_len;
769
770 if (data_len > skb->len)
771 data_len = skb->len;
772
773 size += nla_total_size(data_len);
774 break;
775
776 case NFULNL_COPY_DISABLED:
777 default:
778 goto unlock_and_release;
779 }
780
781 if (inst->skb && size > skb_tailroom(inst->skb)) {
782 /* either the queue len is too high or we don't have
783 * enough room in the skb left. flush to userspace. */
784 __nfulnl_flush(inst);
785 }
786
787 if (!inst->skb) {
788 inst->skb = nfulnl_alloc_skb(net, inst->peer_portid,
789 inst->nlbufsiz, size);
790 if (!inst->skb)
791 goto alloc_failure;
792 }
793
794 inst->qlen++;
795
796 __build_packet_message(log, inst, skb, data_len, pf,
797 hooknum, in, out, prefix, plen,
798 nfnl_ct, ct, ctinfo);
799
800 if (inst->qlen >= qthreshold)
801 __nfulnl_flush(inst);
802 /* timer_pending always called within inst->lock, so there
803 * is no chance of a race here */
804 else if (!timer_pending(&inst->timer)) {
805 instance_get(inst);
806 inst->timer.expires = jiffies + (inst->flushtimeout*HZ/100);
807 add_timer(&inst->timer);
808 }
809
810 unlock_and_release:
811 spin_unlock_bh(&inst->lock);
812 instance_put(inst);
813 return;
814
815 alloc_failure:
816 /* FIXME: statistics */
817 goto unlock_and_release;
818 }
819
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36554 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH nf-next] netfilter: ctnetlink: remove get_ct indirection
2021-01-19 9:21 [PATCH nf-next] netfilter: ctnetlink: remove get_ct indirection Florian Westphal
2021-01-20 14:37 ` kernel test robot
@ 2021-01-20 14:58 ` kernel test robot
1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2021-01-20 14:58 UTC (permalink / raw)
To: Florian Westphal, netfilter-devel; +Cc: kbuild-all, Florian Westphal
[-- Attachment #1: Type: text/plain, Size: 11400 bytes --]
Hi Florian,
I love your patch! Yet something to improve:
[auto build test ERROR on nf-next/master]
url: https://github.com/0day-ci/linux/commits/Florian-Westphal/netfilter-ctnetlink-remove-get_ct-indirection/20210120-190814
base: https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master
config: microblaze-randconfig-r001-20210120 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 9.3.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/0day-ci/linux/commit/20f5f1a1d8f0d775b9982e1404cf44840dd0a86a
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Florian-Westphal/netfilter-ctnetlink-remove-get_ct-indirection/20210120-190814
git checkout 20f5f1a1d8f0d775b9982e1404cf44840dd0a86a
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=microblaze
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
net/netfilter/nfnetlink_queue.c: In function 'nfqnl_build_packet_message':
>> net/netfilter/nfnetlink_queue.c:449:9: error: implicit declaration of function 'nf_ct_get' [-Werror=implicit-function-declaration]
449 | ct = nf_ct_get(entskb, &ctinfo);
| ^~~~~~~~~
>> net/netfilter/nfnetlink_queue.c:449:7: warning: assignment to 'struct nf_conn *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
449 | ct = nf_ct_get(entskb, &ctinfo);
| ^
net/netfilter/nfnetlink_queue.c: In function 'nfqnl_ct_parse':
net/netfilter/nfnetlink_queue.c:1109:5: warning: assignment to 'struct nf_conn *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
1109 | ct = nf_ct_get(entry->skb, ctinfo);
| ^
cc1: some warnings being treated as errors
vim +/nf_ct_get +449 net/netfilter/nfnetlink_queue.c
373
374 static struct sk_buff *
375 nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
376 struct nf_queue_entry *entry,
377 __be32 **packet_id_ptr)
378 {
379 size_t size;
380 size_t data_len = 0, cap_len = 0;
381 unsigned int hlen = 0;
382 struct sk_buff *skb;
383 struct nlattr *nla;
384 struct nfqnl_msg_packet_hdr *pmsg;
385 struct nlmsghdr *nlh;
386 struct nfgenmsg *nfmsg;
387 struct sk_buff *entskb = entry->skb;
388 struct net_device *indev;
389 struct net_device *outdev;
390 struct nf_conn *ct = NULL;
391 enum ip_conntrack_info ctinfo;
392 struct nfnl_ct_hook *nfnl_ct;
393 bool csum_verify;
394 char *secdata = NULL;
395 u32 seclen = 0;
396
397 size = nlmsg_total_size(sizeof(struct nfgenmsg))
398 + nla_total_size(sizeof(struct nfqnl_msg_packet_hdr))
399 + nla_total_size(sizeof(u_int32_t)) /* ifindex */
400 + nla_total_size(sizeof(u_int32_t)) /* ifindex */
401 #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
402 + nla_total_size(sizeof(u_int32_t)) /* ifindex */
403 + nla_total_size(sizeof(u_int32_t)) /* ifindex */
404 #endif
405 + nla_total_size(sizeof(u_int32_t)) /* mark */
406 + nla_total_size(sizeof(struct nfqnl_msg_packet_hw))
407 + nla_total_size(sizeof(u_int32_t)) /* skbinfo */
408 + nla_total_size(sizeof(u_int32_t)); /* cap_len */
409
410 if (entskb->tstamp)
411 size += nla_total_size(sizeof(struct nfqnl_msg_packet_timestamp));
412
413 size += nfqnl_get_bridge_size(entry);
414
415 if (entry->state.hook <= NF_INET_FORWARD ||
416 (entry->state.hook == NF_INET_POST_ROUTING && entskb->sk == NULL))
417 csum_verify = !skb_csum_unnecessary(entskb);
418 else
419 csum_verify = false;
420
421 outdev = entry->state.out;
422
423 switch ((enum nfqnl_config_mode)READ_ONCE(queue->copy_mode)) {
424 case NFQNL_COPY_META:
425 case NFQNL_COPY_NONE:
426 break;
427
428 case NFQNL_COPY_PACKET:
429 if (!(queue->flags & NFQA_CFG_F_GSO) &&
430 entskb->ip_summed == CHECKSUM_PARTIAL &&
431 skb_checksum_help(entskb))
432 return NULL;
433
434 data_len = READ_ONCE(queue->copy_range);
435 if (data_len > entskb->len)
436 data_len = entskb->len;
437
438 hlen = skb_zerocopy_headlen(entskb);
439 hlen = min_t(unsigned int, hlen, data_len);
440 size += sizeof(struct nlattr) + hlen;
441 cap_len = entskb->len;
442 break;
443 }
444
445 nfnl_ct = rcu_dereference(nfnl_ct_hook);
446
447 if (queue->flags & NFQA_CFG_F_CONNTRACK) {
448 if (nfnl_ct != NULL) {
> 449 ct = nf_ct_get(entskb, &ctinfo);
450 if (ct != NULL)
451 size += nfnl_ct->build_size(ct);
452 }
453 }
454
455 if (queue->flags & NFQA_CFG_F_UID_GID) {
456 size += (nla_total_size(sizeof(u_int32_t)) /* uid */
457 + nla_total_size(sizeof(u_int32_t))); /* gid */
458 }
459
460 if ((queue->flags & NFQA_CFG_F_SECCTX) && entskb->sk) {
461 seclen = nfqnl_get_sk_secctx(entskb, &secdata);
462 if (seclen)
463 size += nla_total_size(seclen);
464 }
465
466 skb = alloc_skb(size, GFP_ATOMIC);
467 if (!skb) {
468 skb_tx_error(entskb);
469 goto nlmsg_failure;
470 }
471
472 nlh = nlmsg_put(skb, 0, 0,
473 nfnl_msg_type(NFNL_SUBSYS_QUEUE, NFQNL_MSG_PACKET),
474 sizeof(struct nfgenmsg), 0);
475 if (!nlh) {
476 skb_tx_error(entskb);
477 kfree_skb(skb);
478 goto nlmsg_failure;
479 }
480 nfmsg = nlmsg_data(nlh);
481 nfmsg->nfgen_family = entry->state.pf;
482 nfmsg->version = NFNETLINK_V0;
483 nfmsg->res_id = htons(queue->queue_num);
484
485 nla = __nla_reserve(skb, NFQA_PACKET_HDR, sizeof(*pmsg));
486 pmsg = nla_data(nla);
487 pmsg->hw_protocol = entskb->protocol;
488 pmsg->hook = entry->state.hook;
489 *packet_id_ptr = &pmsg->packet_id;
490
491 indev = entry->state.in;
492 if (indev) {
493 #if !IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
494 if (nla_put_be32(skb, NFQA_IFINDEX_INDEV, htonl(indev->ifindex)))
495 goto nla_put_failure;
496 #else
497 if (entry->state.pf == PF_BRIDGE) {
498 /* Case 1: indev is physical input device, we need to
499 * look for bridge group (when called from
500 * netfilter_bridge) */
501 if (nla_put_be32(skb, NFQA_IFINDEX_PHYSINDEV,
502 htonl(indev->ifindex)) ||
503 /* this is the bridge group "brX" */
504 /* rcu_read_lock()ed by __nf_queue */
505 nla_put_be32(skb, NFQA_IFINDEX_INDEV,
506 htonl(br_port_get_rcu(indev)->br->dev->ifindex)))
507 goto nla_put_failure;
508 } else {
509 int physinif;
510
511 /* Case 2: indev is bridge group, we need to look for
512 * physical device (when called from ipv4) */
513 if (nla_put_be32(skb, NFQA_IFINDEX_INDEV,
514 htonl(indev->ifindex)))
515 goto nla_put_failure;
516
517 physinif = nf_bridge_get_physinif(entskb);
518 if (physinif &&
519 nla_put_be32(skb, NFQA_IFINDEX_PHYSINDEV,
520 htonl(physinif)))
521 goto nla_put_failure;
522 }
523 #endif
524 }
525
526 if (outdev) {
527 #if !IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
528 if (nla_put_be32(skb, NFQA_IFINDEX_OUTDEV, htonl(outdev->ifindex)))
529 goto nla_put_failure;
530 #else
531 if (entry->state.pf == PF_BRIDGE) {
532 /* Case 1: outdev is physical output device, we need to
533 * look for bridge group (when called from
534 * netfilter_bridge) */
535 if (nla_put_be32(skb, NFQA_IFINDEX_PHYSOUTDEV,
536 htonl(outdev->ifindex)) ||
537 /* this is the bridge group "brX" */
538 /* rcu_read_lock()ed by __nf_queue */
539 nla_put_be32(skb, NFQA_IFINDEX_OUTDEV,
540 htonl(br_port_get_rcu(outdev)->br->dev->ifindex)))
541 goto nla_put_failure;
542 } else {
543 int physoutif;
544
545 /* Case 2: outdev is bridge group, we need to look for
546 * physical output device (when called from ipv4) */
547 if (nla_put_be32(skb, NFQA_IFINDEX_OUTDEV,
548 htonl(outdev->ifindex)))
549 goto nla_put_failure;
550
551 physoutif = nf_bridge_get_physoutif(entskb);
552 if (physoutif &&
553 nla_put_be32(skb, NFQA_IFINDEX_PHYSOUTDEV,
554 htonl(physoutif)))
555 goto nla_put_failure;
556 }
557 #endif
558 }
559
560 if (entskb->mark &&
561 nla_put_be32(skb, NFQA_MARK, htonl(entskb->mark)))
562 goto nla_put_failure;
563
564 if (indev && entskb->dev &&
565 entskb->mac_header != entskb->network_header) {
566 struct nfqnl_msg_packet_hw phw;
567 int len;
568
569 memset(&phw, 0, sizeof(phw));
570 len = dev_parse_header(entskb, phw.hw_addr);
571 if (len) {
572 phw.hw_addrlen = htons(len);
573 if (nla_put(skb, NFQA_HWADDR, sizeof(phw), &phw))
574 goto nla_put_failure;
575 }
576 }
577
578 if (nfqnl_put_bridge(entry, skb) < 0)
579 goto nla_put_failure;
580
581 if (entry->state.hook <= NF_INET_FORWARD && entskb->tstamp) {
582 struct nfqnl_msg_packet_timestamp ts;
583 struct timespec64 kts = ktime_to_timespec64(entskb->tstamp);
584
585 ts.sec = cpu_to_be64(kts.tv_sec);
586 ts.usec = cpu_to_be64(kts.tv_nsec / NSEC_PER_USEC);
587
588 if (nla_put(skb, NFQA_TIMESTAMP, sizeof(ts), &ts))
589 goto nla_put_failure;
590 }
591
592 if ((queue->flags & NFQA_CFG_F_UID_GID) && entskb->sk &&
593 nfqnl_put_sk_uidgid(skb, entskb->sk) < 0)
594 goto nla_put_failure;
595
596 if (seclen && nla_put(skb, NFQA_SECCTX, seclen, secdata))
597 goto nla_put_failure;
598
599 if (ct && nfnl_ct->build(skb, ct, ctinfo, NFQA_CT, NFQA_CT_INFO) < 0)
600 goto nla_put_failure;
601
602 if (cap_len > data_len &&
603 nla_put_be32(skb, NFQA_CAP_LEN, htonl(cap_len)))
604 goto nla_put_failure;
605
606 if (nfqnl_put_packet_info(skb, entskb, csum_verify))
607 goto nla_put_failure;
608
609 if (data_len) {
610 struct nlattr *nla;
611
612 if (skb_tailroom(skb) < sizeof(*nla) + hlen)
613 goto nla_put_failure;
614
615 nla = skb_put(skb, sizeof(*nla));
616 nla->nla_type = NFQA_PAYLOAD;
617 nla->nla_len = nla_attr_size(data_len);
618
619 if (skb_zerocopy(skb, entskb, data_len, hlen))
620 goto nla_put_failure;
621 }
622
623 nlh->nlmsg_len = skb->len;
624 if (seclen)
625 security_release_secctx(secdata, seclen);
626 return skb;
627
628 nla_put_failure:
629 skb_tx_error(entskb);
630 kfree_skb(skb);
631 net_err_ratelimited("nf_queue: error creating packet message\n");
632 nlmsg_failure:
633 if (seclen)
634 security_release_secctx(secdata, seclen);
635 return NULL;
636 }
637
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35368 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread