On Wed, Aug 19, 2015 at 3:39 PM, Jesse Gross <je...@nicira.com> wrote:
> On Wed, Aug 19, 2015 at 1:12 PM, Pravin Shelar <pshe...@nicira.com> wrote:
>> 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
>>>> 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.
>
> I think we should still strive to make changes as minimally disruptive
> as possible. At least some changes can still be done safely at runtime
> so it would be nice to be able to handle those cleanly.
>
>>>> +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.
>
> I guess I would rather allow it but make changes slower than disallow
> it. That way it is easier to improve in the future if necessary.
These changes need more refactoring in Geneve code. I will post it as
separate patch series once this series is in.
--
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
