[net 0/2][pull request] Intel Wired LAN Driver Updates 2016-01-25

8 messages, 4 authors, 2016-01-31 · open the first message on its own page

[net 0/2][pull request] Intel Wired LAN Driver Updates 2016-01-25

From: Jeff Kirsher <hidden>
Date: 2016-01-25 23:59:13

This series contains updates to i40e only and so I won't continue receiving
patches to fix the same issue (again).

Arnd fixes the driver from causing the compiler whining about uninitialized
variables, so initialize those variables.

Eric fixes the build errors/warnings which were introduced by Anjali
when she added geneve support to i40e.


The following are changes since commit c85e4924452ae8225c8829f3fa8a2f7baa34bc5c:
  hv_netvsc: Fix book keeping of skb during batching process
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-queue master

Arnd Bergmann (1):
  net: i40e: shut up uninitialized variable warnings

Eric Dumazet (1):
  i40e: fix build warnings

 drivers/net/ethernet/intel/i40e/i40e_main.c | 15 +++++----------
 drivers/net/ethernet/intel/i40e/i40e_txrx.c |  4 ++--
 2 files changed, 7 insertions(+), 12 deletions(-)

-- 
2.5.0

[net 2/2] net: i40e: shut up uninitialized variable warnings

From: Jeff Kirsher <hidden>
Date: 2016-01-25 23:59:14

From: Arnd Bergmann <arnd@arndb.de>

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>
Tested-by: Andrew Bowers <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 720516b..47bd8b3 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) {

-- 
2.5.0

[net 1/2] i40e: fix build warnings

From: Jeff Kirsher <hidden>
Date: 2016-01-25 23:59:14

From: Eric Dumazet <edumazet@google.com>

Fixes following build warnings :

drivers/net/ethernet/intel/i40e/i40e_main.c:7057:13: warning:
'i40e_sync_udp_filters_subtask' defined but not used [-Wunused-function]
drivers/net/ethernet/intel/i40e/i40e_main.c:8524:13: warning:
'i40e_add_vxlan_port' defined but not used [-Wunused-function]
drivers/net/ethernet/intel/i40e/i40e_main.c:8569:13: warning:
'i40e_del_vxlan_port' defined but not used [-Wunused-function]
drivers/net/ethernet/intel/i40e/i40e_main.c:8604:13: warning:
'i40e_add_geneve_port' defined but not used [-Wunused-function]
drivers/net/ethernet/intel/i40e/i40e_main.c:8651:13: warning:
'i40e_del_geneve_port' defined but not used [-Wunused-function]

Fixes: 6a899024058d ("i40e: geneve tunnel offload support")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Tested-by: Andrew Bowers <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index bb4612c..8f3b53e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c

@@ -7117,9 +7117,7 @@ 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
 	i40e_clean_adminq_subtask(pf);
 
 	i40e_service_event_complete(pf);

@@ -8515,6 +8513,8 @@ 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 +8524,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 +8556,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 +8567,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 +8589,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 +8602,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 +8636,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 +8647,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 +8672,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)

-- 
2.5.0

Re: [net 0/2][pull request] Intel Wired LAN Driver Updates 2016-01-25

From: David Miller <davem@davemloft.net>
Date: 2016-01-26 06:52:01

From: Jeff Kirsher <redacted>
Date: Mon, 25 Jan 2016 15:58:50 -0800

This series contains updates to i40e only and so I won't continue receiving
patches to fix the same issue (again).

Arnd fixes the driver from causing the compiler whining about uninitialized
variables, so initialize those variables.

Eric fixes the build errors/warnings which were introduced by Anjali
when she added geneve support to i40e.

Pulled, thanks Jeff.

Re: [net,2/2] net: i40e: shut up uninitialized variable warnings

From: Vasily Averin <hidden>
Date: 2016-01-30 15:34:15

On 26.01.2016 02:58, Jeff Kirsher wrote:

From: Arnd Bergmann <arnd@arndb.de>

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

2364                 if ((tx_ring->flags & I40E_TXR_FLAGS_OUTER_UDP_CSUM) &&
2365                     (l4_tunnel == I40E_TXD_CTX_UDP_TUNNELING)        &&
2366                     (*cd_tunneling & I40E_TXD_CTX_QW0_EXT_IP_MASK)) {
2367                         oudph->check = ~csum_tcpudp_magic(oiph->saddr,
2368                                         oiph->daddr,
2369                                         (skb->len - skb_transport_offset(skb)),
2370                                         IPPROTO_UDP, 0);

if compiler reports that oudph and oiph can be unitialized here,
it's not enough just to set them to NULL.

Do we need probably to check that variables was initialized before access here?
i.e. add  oudph && oiph into condition?

quoted hunk

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Tested-by: Andrew Bowers <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 720516b..47bd8b3 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) {

[PATCH] net: i40e: prevent access to uninitilized variables in i40e_tx_enable_csum()

From: Vasily Averin <hidden>
Date: 2016-01-30 16:48:07

Patch makes safe an access to 'oiph' and 'oudph' variables
if they was not initilized.

Signed-off-by: Vasily Averin <redacted>
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 47bd8b3..779f77e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c

@@ -2363,7 +2363,8 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 		}
 		if ((tx_ring->flags & I40E_TXR_FLAGS_OUTER_UDP_CSUM) &&
 		    (l4_tunnel == I40E_TXD_CTX_UDP_TUNNELING)        &&
-		    (*cd_tunneling & I40E_TXD_CTX_QW0_EXT_IP_MASK)) {
+		    (*cd_tunneling & I40E_TXD_CTX_QW0_EXT_IP_MASK)   &&
+		    oudph && oiph) {
 			oudph->check = ~csum_tcpudp_magic(oiph->saddr,
 					oiph->daddr,
 					(skb->len - skb_transport_offset(skb)),

-- 
1.9.1

Re: [net,2/2] net: i40e: shut up uninitialized variable warnings

From: Arnd Bergmann <arnd@arndb.de>
Date: 2016-01-31 14:59:49

On Saturday 30 January 2016 18:33:26 Vasily Averin wrote:

On 26.01.2016 02:58, Jeff Kirsher wrote:

quoted

From: Arnd Bergmann <arnd@arndb.de>

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

2364                 if ((tx_ring->flags & I40E_TXR_FLAGS_OUTER_UDP_CSUM) &&
2365                     (l4_tunnel == I40E_TXD_CTX_UDP_TUNNELING)        &&
2366                     (*cd_tunneling & I40E_TXD_CTX_QW0_EXT_IP_MASK)) {
2367                         oudph->check = ~csum_tcpudp_magic(oiph->saddr,
2368                                         oiph->daddr,
2369                                         (skb->len - skb_transport_offset(skb)),
2370                                         IPPROTO_UDP, 0);

if compiler reports that oudph and oiph can be unitialized here,
it's not enough just to set them to NULL.

Do we need probably to check that variables was initialized before access here?
i.e. add  oudph && oiph into condition?

Sorry, I should not have mentioned my patch when it wasn't meant as
a serious submission. The patch I sent did not have a proper changelog
on it so it failed to explain it.

The reason why my patch works correctly is that the check on "l4_tunnel
== I40E_TXD_CTX_UDP_TUNNELING" means we can only get here if the
two variables have been initialized, and gcc fails to see this.

Jeff mentioned that he already had a patch for this, so I did not follow
up with a real patch.

	Arnd

Re: [PATCH] net: i40e: prevent access to uninitilized variables in i40e_tx_enable_csum()

From: Arnd Bergmann <arnd@arndb.de>
Date: 2016-01-31 15:00:50

On Saturday 30 January 2016 19:47:36 Vasily Averin wrote:

quoted hunk

Patch makes safe an access to 'oiph' and 'oudph' variables
if they was not initilized.

Signed-off-by: Vasily Averin <redacted>
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 47bd8b3..779f77e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c

@@ -2363,7 +2363,8 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
                }
                if ((tx_ring->flags & I40E_TXR_FLAGS_OUTER_UDP_CSUM) &&
                    (l4_tunnel == I40E_TXD_CTX_UDP_TUNNELING)        &&
-                   (*cd_tunneling & I40E_TXD_CTX_QW0_EXT_IP_MASK)) {
+                   (*cd_tunneling & I40E_TXD_CTX_QW0_EXT_IP_MASK)   &&
+                   oudph && oiph) {
                        oudph->check = ~csum_tcpudp_magic(oiph->saddr,
                                        oiph->daddr,
                                        (skb->len - skb_transport_offset(skb)),

If we can actually get here with oudph==NULL or oiph==NULL, we should back
my patch instead and fix it another way.

In the version we have in net-next, that is not possible.

	Arnd

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help