Re: [PATCH 4/4] macvlan: export macvlan mode through netlink

On Tuesday 24 November 2009, Patrick McHardy wrote:
Arnd Bergmann wrote:
@@ -600,6 +594,18 @@ static int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS])))
return -EADDRNOTAVAIL;
}
+
+ if (data && data[IFLA_MACVLAN_MODE]) {
+ u32 mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
+ switch (mode) {
+ case MACVLAN_MODE_PRIVATE:
+ case MACVLAN_MODE_VEPA:
+ case MACVLAN_MODE_BRIDGE:
+ break;
+ default:
+ return -EINVAL;

EINVAL is quite unspecific. In this case I think EOPNOTSUPP would
be fine and provide more information.

ok

+ }
+ }
return 0;
}
@@ -664,6 +670,13 @@ static int macvlan_newlink(struct net *src_net, struct net_device *dev,
vlan->dev = dev;
vlan->port = port;

+ vlan->mode = MACVLAN_MODE_VEPA;
+ if (data && data[IFLA_MACVLAN_MODE]) {
+ u32 mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
+
+ vlan->mode = mode;

This looks a bit strange, like cut-and-paste without reformatting :)
I'd suggest to simply use "vlan->mode = nla_get_u32(...)".

Yep.

Thanks for the review. Combined changes below.

Arnd <><
---

drivers/net/macvlan.c | 47 +++++++++++++++++++++++------------------------
1 files changed, 23 insertions(+), 24 deletions(-)

--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -118,15 +118,16 @@ static int macvlan_addr_busy(const struct macvlan_port *port,
return 0;
}

-static inline void macvlan_count_rx(const struct macvlan_dev *vlan, int length,
- bool success, bool multicast)
+static inline void macvlan_count_rx(const struct macvlan_dev *vlan,
+ unsigned int len, bool success,
+ bool multicast)
{
struct macvlan_rx_stats *rx_stats;

rx_stats = per_cpu_ptr(vlan->rx_stats, smp_processor_id());
if (likely(success)) {
rx_stats->rx_packets++;;
- rx_stats->rx_bytes += length;
+ rx_stats->rx_bytes += len;
if (multicast)
rx_stats->multicast++;
} else {
@@ -170,7 +171,7 @@ static void macvlan_broadcast(struct sk_buff *skb,

for (i = 0; i < MACVLAN_HASH_SIZE; i++) {
hlist_for_each_entry_rcu(vlan, n, &port->vlan_hash[i], hlist) {
- if ((vlan->dev == src) || !(vlan->mode & mode))
+ if (vlan->dev == src || !(vlan->mode & mode))
continue;

nskb = skb_clone(skb, GFP_ATOMIC);
@@ -190,7 +191,7 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
const struct macvlan_dev *vlan;
const struct macvlan_dev *src;
struct net_device *dev;
- int len;
+ unsigned int len;

port = rcu_dereference(skb->dev->macvlan_port);
if (port == NULL)
@@ -200,17 +201,22 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
src = macvlan_hash_lookup(port, eth->h_source);
if (!src)
/* frame comes from an external address */
- macvlan_broadcast(skb, port, NULL, MACVLAN_MODE_PRIVATE
- | MACVLAN_MODE_VEPA | MACVLAN_MODE_BRIDGE);
+ macvlan_broadcast(skb, port, NULL,
+ MACVLAN_MODE_PRIVATE |
+ MACVLAN_MODE_VEPA |
+ MACVLAN_MODE_BRIDGE);
else if (src->mode == MACVLAN_MODE_VEPA)
/* flood to everyone except source */
macvlan_broadcast(skb, port, src->dev,
- MACVLAN_MODE_VEPA | MACVLAN_MODE_BRIDGE);
+ MACVLAN_MODE_VEPA |
+ MACVLAN_MODE_BRIDGE);
else if (src->mode == MACVLAN_MODE_BRIDGE)
- /* flood only to VEPA ports, bridge ports
- already saw the frame */
+ /*
+ * flood only to VEPA ports, bridge ports
+ * already saw the frame on the way out.
+ */
macvlan_broadcast(skb, port, src->dev,
- MACVLAN_MODE_VEPA);
+ MACVLAN_MODE_VEPA);
return skb;
}

@@ -253,7 +259,7 @@ static int macvlan_queue_xmit(struct sk_buff *skb, struct net_device *dev)

dest = macvlan_hash_lookup(port, eth->h_dest);
if (dest && dest->mode == MACVLAN_MODE_BRIDGE) {
- int length = skb->len + ETH_HLEN;
+ unsigned int length = skb->len + ETH_HLEN;
int ret = dev_forward_skb(dest->dev, skb);
macvlan_count_rx(dest, length,
ret == NET_RX_SUCCESS, 0);
@@ -604,8 +610,7 @@ static int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
}

if (data && data[IFLA_MACVLAN_MODE]) {
- u32 mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
- switch (mode) {
+ switch (nla_get_u32(data[IFLA_MACVLAN_MODE])) {
case MACVLAN_MODE_PRIVATE:
case MACVLAN_MODE_VEPA:
case MACVLAN_MODE_BRIDGE:
@@ -679,11 +684,8 @@ static int macvlan_newlink(struct net *src_net, struct net_device *dev,
vlan->port = port;

vlan->mode = MACVLAN_MODE_VEPA;
- if (data && data[IFLA_MACVLAN_MODE]) {
- u32 mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
-
- vlan->mode = mode;
- }
+ if (data && data[IFLA_MACVLAN_MODE])
+ vlan->mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);

err = register_netdevice(dev);
if (err < 0)
@@ -710,11 +712,8 @@ static int macvlan_changelink(struct net_device *dev,
struct nlattr *tb[], struct nlattr *data[])
{
struct macvlan_dev *vlan = netdev_priv(dev);
- if (data && data[IFLA_MACVLAN_MODE]) {
- u32 mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
- vlan->mode = mode;
- }
-
+ if (data && data[IFLA_MACVLAN_MODE])
+ vlan->mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
return 0;
}

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Relevant Pages