[PATCH next 2/3] ipvlan: mode is u16

Subsystems: networking drivers, the rest

STALE3834d

3 messages, 3 authors, 2016-02-08 · open the first message on its own page

[PATCH next 2/3] ipvlan: mode is u16

From: Mahesh Bandewar <hidden>
Date: 2016-02-02 19:20:44

From: Mahesh Bandewar <redacted>

The mode argument was erronusly defined as u32 but it has always
been u16.

Signed-off-by: Mahesh Bandewar <redacted>
---
 drivers/net/ipvlan/ipvlan.h      | 1 -
 drivers/net/ipvlan/ipvlan_main.c | 9 ++++++---
 2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ipvlan/ipvlan.h b/drivers/net/ipvlan/ipvlan.h
index 9542b7bac61a..817cab1a7959 100644
--- a/drivers/net/ipvlan/ipvlan.h
+++ b/drivers/net/ipvlan/ipvlan.h
@@ -115,7 +115,6 @@ static inline struct ipvl_port *ipvlan_port_get_rtnl(const struct net_device *d)
 }
 
 void ipvlan_adjust_mtu(struct ipvl_dev *ipvlan, struct net_device *dev);
-void ipvlan_set_port_mode(struct ipvl_port *port, u32 nval);
 void ipvlan_init_secret(void);
 unsigned int ipvlan_mac_hash(const unsigned char *addr);
 rx_handler_result_t ipvlan_handle_frame(struct sk_buff **pskb);
diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index f94392d07126..54c542526262 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -14,7 +14,7 @@ void ipvlan_adjust_mtu(struct ipvl_dev *ipvlan, struct net_device *dev)
 	ipvlan->dev->mtu = dev->mtu - ipvlan->mtu_adj;
 }
 
-void ipvlan_set_port_mode(struct ipvl_port *port, u32 nval)
+static void ipvlan_set_port_mode(struct ipvl_port *port, u16 nval)
 {
 	struct ipvl_dev *ipvlan;
 
@@ -442,6 +442,7 @@ static int ipvlan_link_new(struct net *src_net, struct net_device *dev,
 	struct ipvl_port *port;
 	struct net_device *phy_dev;
 	int err;
+	u16 mode = IPVLAN_MODE_L3;
 
 	if (!tb[IFLA_LINK])
 		return -EINVAL;
@@ -460,10 +461,10 @@ static int ipvlan_link_new(struct net *src_net, struct net_device *dev,
 			return err;
 	}
 
-	port = ipvlan_port_get_rtnl(phy_dev);
 	if (data && data[IFLA_IPVLAN_MODE])
-		port->mode = nla_get_u16(data[IFLA_IPVLAN_MODE]);
+		mode = nla_get_u16(data[IFLA_IPVLAN_MODE]);
 
+	port = ipvlan_port_get_rtnl(phy_dev);
 	ipvlan->phy_dev = phy_dev;
 	ipvlan->dev = dev;
 	ipvlan->port = port;
@@ -488,6 +489,8 @@ static int ipvlan_link_new(struct net *src_net, struct net_device *dev,
 		goto ipvlan_destroy_port;
 
 	list_add_tail_rcu(&ipvlan->pnode, &port->ipvlans);
+	ipvlan_set_port_mode(port, mode);
+
 	netif_stacked_transfer_operstate(phy_dev, dev);
 	return 0;
 
-- 
2.7.0.rc3.207.g0ac5344

Re: [PATCH next 2/3] ipvlan: mode is u16

From: David Miller <davem@davemloft.net>
Date: 2016-02-07 19:19:14

From: Mahesh Bandewar <redacted>
Date: Tue,  2 Feb 2016 11:20:30 -0800
From: Mahesh Bandewar <redacted>

The mode argument was erronusly defined as u32 but it has always
been u16.

Signed-off-by: Mahesh Bandewar <redacted>
This patch is changing more than this.

Specifically it is invoking ipvlan_set_port_mode() instead of assigning
port->mode directly in ipvlan_link_new().

That minimally needs to be documented in the commit log message.

But I also wonder if for some reason in this conect the direct
assignment happens to be fine.  That's pretty much why you should
explain things.

Re: [PATCH next 2/3] ipvlan: mode is u16

From: Mahesh Bandewar <hidden>
Date: 2016-02-08 22:15:14

On Sun, Feb 7, 2016 at 11:19 AM, David Miller [off-list ref] wrote:
From: Mahesh Bandewar <redacted>
Date: Tue,  2 Feb 2016 11:20:30 -0800
quoted
From: Mahesh Bandewar <redacted>

The mode argument was erronusly defined as u32 but it has always
been u16.

Signed-off-by: Mahesh Bandewar <redacted>
This patch is changing more than this.

Specifically it is invoking ipvlan_set_port_mode() instead of assigning
port->mode directly in ipvlan_link_new().

That minimally needs to be documented in the commit log message.

But I also wonder if for some reason in this conect the direct
assignment happens to be fine.  That's pretty much why you should
explain things.
Directly assigning does serve the same purpose, however when there is
a helper to do just that (and other things in future) it would help
keep all related things at one place avoiding errors in future. Since
it's a minor improvement I did not mention that into the commit log.

I'll update the commit log and resend.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help