| Message ID | 2806620.EpZqDlZTO3@wuerfel |
|---|---|
| State | New |
| Headers | show |
On Wed, 2016-01-20 at 11:42 +0100, Arnd Bergmann wrote: > The addition of the geneve tunnel offload code left a couple > of functions unconditionally defined but empty whenever CONFIG_VXLAN > and CONFIG_GENEVE are disabled. gcc warns about this: > > i40e_main.c:7049:13: warning: 'i40e_sync_udp_filters_subtask' defined > but not used [-Wunused-function] > i40e_main.c:8516:13: warning: 'i40e_add_vxlan_port' defined but not > used [-Wunused-function] > i40e_main.c:8561:13: warning: 'i40e_del_vxlan_port' defined but not > used [-Wunused-function] > i40e_main.c:8596:13: warning: 'i40e_add_geneve_port' defined but not > used [-Wunused-function] > i40e_main.c:8643:13: warning: 'i40e_del_geneve_port' defined but not > used [-Wunused-function] > > This moves the #ifdef statements to the outside of the affected > functions, which avoids the warnings. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > Fixes: 6a899024058d ("i40e: geneve tunnel offload support") > --- > This is a harmless regression against v4.4, found on ARM randconfig > builds Thanks Arnd, I already have a patch from Eric Dumazet and Alex Duyck to resolve this issue. Dave- I plan on pushing the fix later today to net.
On Wednesday 20 January 2016 14:17:25 Jeff Kirsher wrote: > On Wed, 2016-01-20 at 11:42 +0100, Arnd Bergmann wrote: > > The addition of the geneve tunnel offload code left a couple > > of functions unconditionally defined but empty whenever CONFIG_VXLAN > > and CONFIG_GENEVE are disabled. gcc warns about this: > > > > i40e_main.c:7049:13: warning: 'i40e_sync_udp_filters_subtask' defined > > but not used [-Wunused-function] > > i40e_main.c:8516:13: warning: 'i40e_add_vxlan_port' defined but not > > used [-Wunused-function] > > i40e_main.c:8561:13: warning: 'i40e_del_vxlan_port' defined but not > > used [-Wunused-function] > > i40e_main.c:8596:13: warning: 'i40e_add_geneve_port' defined but not > > used [-Wunused-function] > > i40e_main.c:8643:13: warning: 'i40e_del_geneve_port' defined but not > > used [-Wunused-function] > > > > This moves the #ifdef statements to the outside of the affected > > functions, which avoids the warnings. > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > Fixes: 6a899024058d ("i40e: geneve tunnel offload support") > > --- > > This is a harmless regression against v4.4, found on ARM randconfig > > builds > > Thanks Arnd, I already have a patch from Eric Dumazet and Alex Duyck to > resolve this issue. > > Dave- I plan on pushing the fix later today to net. Ok, thanks. FWIW, I have another patch for this driver that I did not yet submit because it's not a regression and I haven't written a proper changelog for it (it's in a set of 15 patches for netdev that fix harmless warnings). Do you have one for the warning below as well? I could not come up with a better way than adding a bogus initialization, but maybe there is one. Arnd From d89be3f0f932a71dfb5480ee396db514879097c4 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann <arnd@arndb.de> Date: Fri, 15 Jan 2016 17:31:07 +0100 Subject: [PATCH] net: i40e: shut up uninitialized variable warnings intel/i40e/i40e_txrx.c: In function 'i40e_xmit_frame_ring': intel/i40e/i40e_txrx.c:2367:20: error: 'oiph' may be used uninitialized in this function [-Werror=maybe-uninitialized] intel/i40e/i40e_txrx.c:2317:16: note: 'oiph' was declared here intel/i40e/i40e_txrx.c:2367:17: error: 'oudph' may be used uninitialized in this function [-Werror=maybe-uninitialized] intel/i40e/i40e_txrx.c:2316:17: note: 'oudph' was declared here Signed-off-by: Arnd Bergmann <arnd@arndb.de>diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c index 720516b0e8ee..47bd8b3145a7 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c @@ -2313,8 +2313,8 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags, struct iphdr *this_ip_hdr; u32 network_hdr_len; u8 l4_hdr = 0; - struct udphdr *oudph; - struct iphdr *oiph; + struct udphdr *oudph = NULL; + struct iphdr *oiph = NULL; u32 l4_tunnel = 0; if (skb->encapsulation) {
On Wed, 2016-01-20 at 23:28 +0100, Arnd Bergmann wrote: > On Wednesday 20 January 2016 14:17:25 Jeff Kirsher wrote: > > On Wed, 2016-01-20 at 11:42 +0100, Arnd Bergmann wrote: > > > The addition of the geneve tunnel offload code left a couple > > > of functions unconditionally defined but empty whenever > CONFIG_VXLAN > > > and CONFIG_GENEVE are disabled. gcc warns about this: > > > > > > i40e_main.c:7049:13: warning: 'i40e_sync_udp_filters_subtask' > defined > > > but not used [-Wunused-function] > > > i40e_main.c:8516:13: warning: 'i40e_add_vxlan_port' defined but > not > > > used [-Wunused-function] > > > i40e_main.c:8561:13: warning: 'i40e_del_vxlan_port' defined but > not > > > used [-Wunused-function] > > > i40e_main.c:8596:13: warning: 'i40e_add_geneve_port' defined but > not > > > used [-Wunused-function] > > > i40e_main.c:8643:13: warning: 'i40e_del_geneve_port' defined but > not > > > used [-Wunused-function] > > > > > > This moves the #ifdef statements to the outside of the affected > > > functions, which avoids the warnings. > > > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > > Fixes: 6a899024058d ("i40e: geneve tunnel offload support") > > > --- > > > This is a harmless regression against v4.4, found on ARM > randconfig > > > builds > > > > Thanks Arnd, I already have a patch from Eric Dumazet and Alex > Duyck to > > resolve this issue. > > > > Dave- I plan on pushing the fix later today to net. > > Ok, thanks. > > FWIW, I have another patch for this driver that I did not yet submit > because it's not a regression and I haven't written a proper > changelog > for it (it's in a set of 15 patches for netdev that fix harmless > warnings). > > Do you have one for the warning below as well? I could not come up > with a better way than adding a bogus initialization, but maybe > there is one. > > Arnd > > From d89be3f0f932a71dfb5480ee396db514879097c4 Mon Sep 17 00:00:00 > 2001 > From: Arnd Bergmann <arnd@arndb.de> > Date: Fri, 15 Jan 2016 17:31:07 +0100 > Subject: [PATCH] net: i40e: shut up uninitialized variable warnings > > intel/i40e/i40e_txrx.c: In function 'i40e_xmit_frame_ring': > intel/i40e/i40e_txrx.c:2367:20: error: 'oiph' may be used > uninitialized in this function [-Werror=maybe-uninitialized] > intel/i40e/i40e_txrx.c:2317:16: note: 'oiph' was declared here > intel/i40e/i40e_txrx.c:2367:17: error: 'oudph' may be used > uninitialized in this function [-Werror=maybe-uninitialized] > intel/i40e/i40e_txrx.c:2316:17: note: 'oudph' was declared here > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Yeah, I have a fix for that as well. You can confirm by pulling my next-queue tree (dev-queue branch). git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue.git d ev-queue
On Wednesday 20 January 2016 14:44:45 Jeff Kirsher wrote: > Yeah, I have a fix for that as well. > > You can confirm by pulling my next-queue tree (dev-queue branch). > git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue.git d > ev-queue > I checked out that branch, but still see both warnings in that one, plus a new build error in igb, which I don't see in linux-next: drivers/net/ethernet/intel/igb/igb_cdev.c: In function 'igb_mapring': drivers/net/ethernet/intel/igb/igb_cdev.c:150:2: error: implicit declaration of function 'set_pages_uc' [-Werror=implicit-function-declaration] set_pages_uc(virt_to_page(ring->desc), ring->size >> PAGE_SHIFT); ^ drivers/net/ethernet/intel/igb/igb_cdev.c: In function 'igb_unmapring': drivers/net/ethernet/intel/igb/igb_cdev.c:275:2: error: implicit declaration of function 'set_pages_wb' [-Werror=implicit-function-declaration] set_pages_wb(virt_to_page(ring->desc), ring->size >> PAGE_SHIFT); Arnd
On Wed, 2016-01-20 at 23:54 +0100, Arnd Bergmann wrote: > On Wednesday 20 January 2016 14:44:45 Jeff Kirsher wrote: > > Yeah, I have a fix for that as well. > > > > You can confirm by pulling my next-queue tree (dev-queue branch). > > git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next- > queue.git d > > ev-queue > > > > I checked out that branch, but still see both warnings in that one, > plus > a new build error in igb, which I don't see in linux-next: > > drivers/net/ethernet/intel/igb/igb_cdev.c: In function 'igb_mapring': > drivers/net/ethernet/intel/igb/igb_cdev.c:150:2: error: implicit > declaration of function 'set_pages_uc' [-Werror=implicit-function- > declaration] > set_pages_uc(virt_to_page(ring->desc), ring->size >> PAGE_SHIFT); > ^ > drivers/net/ethernet/intel/igb/igb_cdev.c: In function > 'igb_unmapring': > drivers/net/ethernet/intel/igb/igb_cdev.c:275:2: error: implicit > declaration of function 'set_pages_wb' [-Werror=implicit-function- > declaration] > set_pages_wb(virt_to_page(ring->desc), ring->size >> PAGE_SHIFT); Oops, I just realized I had not pushed my latest tree to kernel.org. The igb issue still remains, I am working with the developer who introduced the issue. Looks like the i40e issue about possible uninitialized variables still exists. I thought we had resolved that issue, but apparently not. You should see Eric Dumazet's patch on the tree to resolve the other i40e build warnings. I can add your second patch to resolve the uninitialized variables to my tree.
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index bb4612c159fd..49ab0426b773 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -7056,7 +7056,6 @@ static void i40e_handle_mdd_event(struct i40e_pf *pf) **/ static void i40e_sync_udp_filters_subtask(struct i40e_pf *pf) { -#if IS_ENABLED(CONFIG_VXLAN) || IS_ENABLED(CONFIG_GENEVE) struct i40e_hw *hw = &pf->hw; i40e_status ret; __be16 port; @@ -7090,7 +7089,6 @@ static void i40e_sync_udp_filters_subtask(struct i40e_pf *pf) } } } -#endif } /** @@ -7117,9 +7115,8 @@ static void i40e_service_task(struct work_struct *work) i40e_watchdog_subtask(pf); i40e_fdir_reinit_subtask(pf); i40e_sync_filters_subtask(pf); -#if IS_ENABLED(CONFIG_VXLAN) || IS_ENABLED(CONFIG_GENEVE) - i40e_sync_udp_filters_subtask(pf); -#endif + if (IS_ENABLED(CONFIG_VXLAN) || IS_ENABLED(CONFIG_GENEVE)) + i40e_sync_udp_filters_subtask(pf); i40e_clean_adminq_subtask(pf); i40e_service_event_complete(pf); @@ -8515,6 +8512,7 @@ static u8 i40e_get_udp_port_idx(struct i40e_pf *pf, __be16 port) } #endif +#if IS_ENABLED(CONFIG_VXLAN) /** * i40e_add_vxlan_port - Get notifications about VXLAN ports that come up * @netdev: This physical port's netdev @@ -8524,7 +8522,6 @@ static u8 i40e_get_udp_port_idx(struct i40e_pf *pf, __be16 port) static void i40e_add_vxlan_port(struct net_device *netdev, sa_family_t sa_family, __be16 port) { -#if IS_ENABLED(CONFIG_VXLAN) struct i40e_netdev_priv *np = netdev_priv(netdev); struct i40e_vsi *vsi = np->vsi; struct i40e_pf *pf = vsi->back; @@ -8557,7 +8554,6 @@ static void i40e_add_vxlan_port(struct net_device *netdev, pf->udp_ports[next_idx].type = I40E_AQC_TUNNEL_TYPE_VXLAN; pf->pending_udp_bitmap |= BIT_ULL(next_idx); pf->flags |= I40E_FLAG_UDP_FILTER_SYNC; -#endif } /** @@ -8569,7 +8565,6 @@ static void i40e_add_vxlan_port(struct net_device *netdev, static void i40e_del_vxlan_port(struct net_device *netdev, sa_family_t sa_family, __be16 port) { -#if IS_ENABLED(CONFIG_VXLAN) struct i40e_netdev_priv *np = netdev_priv(netdev); struct i40e_vsi *vsi = np->vsi; struct i40e_pf *pf = vsi->back; @@ -8592,9 +8587,10 @@ static void i40e_del_vxlan_port(struct net_device *netdev, netdev_warn(netdev, "vxlan port %d was not found, not deleting\n", ntohs(port)); } -#endif } +#endif +#if IS_ENABLED(CONFIG_GENEVE) /** * i40e_add_geneve_port - Get notifications about GENEVE ports that come up * @netdev: This physical port's netdev @@ -8604,7 +8600,6 @@ static void i40e_del_vxlan_port(struct net_device *netdev, static void i40e_add_geneve_port(struct net_device *netdev, sa_family_t sa_family, __be16 port) { -#if IS_ENABLED(CONFIG_GENEVE) struct i40e_netdev_priv *np = netdev_priv(netdev); struct i40e_vsi *vsi = np->vsi; struct i40e_pf *pf = vsi->back; @@ -8639,7 +8634,6 @@ static void i40e_add_geneve_port(struct net_device *netdev, pf->flags |= I40E_FLAG_UDP_FILTER_SYNC; dev_info(&pf->pdev->dev, "adding geneve port %d\n", ntohs(port)); -#endif } /** @@ -8651,7 +8645,6 @@ static void i40e_add_geneve_port(struct net_device *netdev, static void i40e_del_geneve_port(struct net_device *netdev, sa_family_t sa_family, __be16 port) { -#if IS_ENABLED(CONFIG_GENEVE) struct i40e_netdev_priv *np = netdev_priv(netdev); struct i40e_vsi *vsi = np->vsi; struct i40e_pf *pf = vsi->back; @@ -8677,8 +8670,8 @@ static void i40e_del_geneve_port(struct net_device *netdev, netdev_warn(netdev, "geneve port %d was not found, not deleting\n", ntohs(port)); } -#endif } +#endif static int i40e_get_phys_port_id(struct net_device *netdev, struct netdev_phys_item_id *ppid)
The addition of the geneve tunnel offload code left a couple of functions unconditionally defined but empty whenever CONFIG_VXLAN and CONFIG_GENEVE are disabled. gcc warns about this: i40e_main.c:7049:13: warning: 'i40e_sync_udp_filters_subtask' defined but not used [-Wunused-function] i40e_main.c:8516:13: warning: 'i40e_add_vxlan_port' defined but not used [-Wunused-function] i40e_main.c:8561:13: warning: 'i40e_del_vxlan_port' defined but not used [-Wunused-function] i40e_main.c:8596:13: warning: 'i40e_add_geneve_port' defined but not used [-Wunused-function] i40e_main.c:8643:13: warning: 'i40e_del_geneve_port' defined but not used [-Wunused-function] This moves the #ifdef statements to the outside of the affected functions, which avoids the warnings. Signed-off-by: Arnd Bergmann <arnd@arndb.de> Fixes: 6a899024058d ("i40e: geneve tunnel offload support") --- This is a harmless regression against v4.4, found on ARM randconfig builds