On Wed, Aug 19, 2015 at 12:40 PM, Jesse Gross <je...@nicira.com> wrote: > On Mon, Aug 17, 2015 at 2:11 PM, Pravin B Shelar <pshe...@nicira.com> wrote: >> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c >> index e47cdd9..0d7fbef 100644 >> --- a/drivers/net/geneve.c >> +++ b/drivers/net/geneve.c >> -static int geneve_configure(struct net *net, struct net_device *dev, >> - __be32 rem_addr, __u32 vni, __u8 ttl, __u8 tos, >> - __u16 dst_port, bool metadata) >> +static int __geneve_configure(struct net *net, struct net_device *dev, >> + __be32 rem_addr, __u32 vni, __u8 ttl, __u8 tos, >> + __u16 dst_port, bool metadata) >> { > [...] >> geneve->net = net; >> geneve->dev = dev; > > I guess this stuff should really be in geneve_configure() - it seems a > bit odd to change it for a running device (even if it shouldn't > change). > ok.
>> geneve->remote.sin_addr.s_addr = rem_addr; >> if (IN_MULTICAST(ntohl(geneve->remote.sin_addr.s_addr))) >> return -EINVAL; >> >> + u32_to_vni(vni, geneve->vni); >> list_for_each_entry(t, &gn->geneve_list, next) { >> if (!memcmp(geneve->vni, t->vni, sizeof(t->vni)) && >> rem_addr == t->remote.sin_addr.s_addr && > > I'm not sure that these types of operations are safe if the device is > already running. We first overwrite the remote value and then we do > error checking but that means that if there is an error, then the > device will be left in a broken state. Don't we also need to update > the hash table if some of these parameters change? > ok, I will stop device before making changes. that way we can add it to hash table. >> +static int geneve_changelink(struct net_device *dev, >> + struct nlattr *tb[], struct nlattr *data[]) >> +{ > [...] >> - if (data[IFLA_GENEVE_PORT]) >> - dst_port = nla_get_u16(data[IFLA_GENEVE_PORT]); >> + if (geneve->sock && (dst_port != ntohs(geneve->dst_port) || >> + metadata != geneve->collect_md)) { > > It seems like in an ideal world, we wouldn't need to recreate the > socket if metadata collection changed (assuming that there are no new > conflicts). To keep changelink simple I am thinking of disallowing metadata changes. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html