[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEP_g=9biFJDxbAFMBB+mbttBnbEkykJx7z24HrhUdjA25YpNg@mail.gmail.com>
Date: Wed, 12 Aug 2015 10:00:19 -0700
From: Jesse Gross <jesse@...ira.com>
To: Pravin B Shelar <pshelar@...ira.com>
Cc: netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 2/6] geneve: Make dst-port configurable.
On Tue, Aug 11, 2015 at 10:17 PM, Pravin B Shelar <pshelar@...ira.com> wrote:
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index 78d49d1..5e9bab8 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -378,6 +379,11 @@ static int geneve_newlink(struct net *net, struct net_device *dev,
> if (data[IFLA_GENEVE_TOS])
> geneve->tos = nla_get_u8(data[IFLA_GENEVE_TOS]);
>
> + if (data[IFLA_GENEVE_PORT])
> + geneve->dst_port = htons(nla_get_u16(data[IFLA_GENEVE_PORT]));
> + else
> + geneve->dst_port = htons(GENEVE_UDP_PORT);
I think there is a looming compatibility problem: if the request for
an alternate port isn't successful then it will be silently ignored,
which would presumably be confusing to the user when things appeared
to have worked without an error. Perhaps the right thing to do is to
check to see whether you get it back when the configuration is dumped
but it is worth thinking about now.
Presumably IFLA_GENEVE_PORT should be added to geneve_policy.
Not specific to this patch but I think that it will make things
significantly easier in the future if changelink was implemented.
VXLAN has the same issue as well.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists