netdev - Re: [PATCH net-next v4 4/4] act_mirred: use TC_ACT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <201807291008.mX0WDfbR%fengguang.wu@intel.com>
Date:   Sun, 29 Jul 2018 10:58:33 +0800
From:   kbuild test robot <lkp@...el.com>
To:     Paolo Abeni <pabeni@...hat.com>
Cc:     kbuild-all@...org, netdev@...r.kernel.org,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Jiri Pirko <jiri@...nulli.us>,
        Daniel Borkmann <daniel@...earbox.net>,
        Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
        Eyal Birger <eyal.birger@...il.com>,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH net-next v4 4/4] act_mirred: use TC_ACT_REINSERT when
 possible

Hi Paolo,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Paolo-Abeni/TC-refactor-act_mirred-packets-re-injection/20180729-102154
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.1.0 make.cross ARCH=xtensa 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   net/sched/act_mirred.c: In function 'tcf_mirred':
>> net/sched/act_mirred.c:268:6: warning: 'is_redirect' may be used uninitialized in this function [-Wmaybe-uninitialized]
      if (is_redirect)
         ^

vim +/is_redirect +268 net/sched/act_mirred.c

   182	
   183	static int tcf_mirred(struct sk_buff *skb, const struct tc_action *a,
   184			      struct tcf_result *res)
   185	{
   186		struct tcf_mirred *m = to_mirred(a);
   187		struct sk_buff *skb2 = skb;
   188		bool m_mac_header_xmit;
   189		struct net_device *dev;
   190		int retval, err = 0;
   191		bool use_reinsert;
   192		bool want_ingress;
   193		bool is_redirect;
   194		int m_eaction;
   195		int mac_len;
   196	
   197		tcf_lastuse_update(&m->tcf_tm);
   198		bstats_cpu_update(this_cpu_ptr(m->common.cpu_bstats), skb);
   199	
   200		m_mac_header_xmit = READ_ONCE(m->tcfm_mac_header_xmit);
   201		m_eaction = READ_ONCE(m->tcfm_eaction);
   202		retval = READ_ONCE(m->tcf_action);
   203		dev = rcu_dereference_bh(m->tcfm_dev);
   204		if (unlikely(!dev)) {
   205			pr_notice_once("tc mirred: target device is gone\n");
   206			goto out;
   207		}
   208	
   209		if (unlikely(!(dev->flags & IFF_UP))) {
   210			net_notice_ratelimited("tc mirred to Houston: device %s is down\n",
   211					       dev->name);
   212			goto out;
   213		}
   214	
   215		/* we could easily avoid the clone only if called by ingress and clsact;
   216		 * since we can't easily detect the clsact caller, skip clone only for
   217		 * ingress - that covers the TC S/W datapath.
   218		 */
   219		is_redirect = tcf_mirred_is_act_redirect(m_eaction);
   220		use_reinsert = skb_at_tc_ingress(skb) && is_redirect &&
   221			       tcf_mirred_can_reinsert(retval);
   222		if (!use_reinsert) {
   223			skb2 = skb_clone(skb, GFP_ATOMIC);
   224			if (!skb2)
   225				goto out;
   226		}
   227	
   228		/* If action's target direction differs than filter's direction,
   229		 * and devices expect a mac header on xmit, then mac push/pull is
   230		 * needed.
   231		 */
   232		want_ingress = tcf_mirred_act_wants_ingress(m_eaction);
   233		if (skb_at_tc_ingress(skb) != want_ingress && m_mac_header_xmit) {
   234			if (!skb_at_tc_ingress(skb)) {
   235				/* caught at egress, act ingress: pull mac */
   236				mac_len = skb_network_header(skb) - skb_mac_header(skb);
   237				skb_pull_rcsum(skb2, mac_len);
   238			} else {
   239				/* caught at ingress, act egress: push mac */
   240				skb_push_rcsum(skb2, skb->mac_len);
   241			}
   242		}
   243	
   244		skb2->skb_iif = skb->dev->ifindex;
   245		skb2->dev = dev;
   246	
   247		/* mirror is always swallowed */
   248		if (is_redirect) {
   249			skb2->tc_redirected = 1;
   250			skb2->tc_from_ingress = skb2->tc_at_ingress;
   251	
   252			/* let's the caller reinsert the packet, if possible */
   253			if (use_reinsert) {
   254				res->ingress = want_ingress;
   255				res->qstats = this_cpu_ptr(m->common.cpu_qstats);
   256				return TC_ACT_REINSERT;
   257			}
   258		}
   259	
   260		if (!want_ingress)
   261			err = dev_queue_xmit(skb2);
   262		else
   263			err = netif_receive_skb(skb2);
   264	
   265		if (err) {
   266	out:
   267			qstats_overlimit_inc(this_cpu_ptr(m->common.cpu_qstats));
 > 268			if (is_redirect)
   269				retval = TC_ACT_SHOT;
   270		}
   271	
   272		return retval;
   273	}
   274	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Download attachment ".config.gz" of type "application/gzip" (54195 bytes)