On Tue, Nov 7, 2017 at 12:33 AM, Xin Long <lucien....@gmail.com> wrote:
> Now ip_gre is using ip_tunnel_changelink to update it's properties, but
> ip_tunnel_changelink in ip_tunnel doesn't update i/o_flags as a common
> function.
>
> o_flags updates would cause that tunnel->tun_hlen / hlen and dev->mtu /
> needed_headroom need to be recalculated, and dev->(hw_)features need to
> be updated as well.
>
> Therefore, we can't just add the update into ip_tunnel_update called
> in ip_tunnel_changelink, and it's also better not to touch ip_tunnel
> codes.
>
> This patch updates i/o_flags and calls ipgre_link_update to recalculate
> these gre properties after ip_tunnel_changelink does the common update.
>
> Note that since ipgre_link_update doesn't know the lower dev, it will
> update gre->hlen, dev->mtu and dev->needed_headroom with the value of
> 'new tun_hlen - old tun_hlen'. In this way, we can avoid many redundant
> codes, unlike ip6_gre.
>
> Reported-by: Jianlin Shi <ji...@redhat.com>
> Signed-off-by: Xin Long <lucien....@gmail.com>
> ---
> net/ipv4/ip_gre.c | 39 +++++++++++++++++++++++++++++++++++++--
> 1 file changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index c105a31..81e1e20 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -773,6 +773,30 @@ static netdev_tx_t gre_tap_xmit(struct sk_buff *skb,
> return NETDEV_TX_OK;
> }
>
> +static void ipgre_link_update(struct net_device *dev, bool set_mtu)
> +{
> + struct ip_tunnel *tunnel = netdev_priv(dev);
> + int len;
> +
> + len = tunnel->tun_hlen;
> + tunnel->tun_hlen = gre_calc_hlen(tunnel->parms.o_flags);
> + len = tunnel->tun_hlen - len;
> + tunnel->hlen = tunnel->hlen + len;
> +
> + dev->needed_headroom = dev->needed_headroom + len;
> + if (set_mtu)
> + dev->mtu = max_t(int, dev->mtu - len, 68);
> +
> + if (!(tunnel->parms.o_flags & TUNNEL_SEQ)) {
> + if (!(tunnel->parms.o_flags & TUNNEL_CSUM) ||
> + tunnel->encap.type == TUNNEL_ENCAP_NONE) {
> + dev->features |= NETIF_F_GSO_SOFTWARE;
> + dev->hw_features |= NETIF_F_GSO_SOFTWARE;
> + }
> + dev->features |= NETIF_F_LLTX;
> + }
> +}
> +
> static int ipgre_tunnel_ioctl(struct net_device *dev,
> struct ifreq *ifr, int cmd)
> {
> @@ -1307,9 +1331,9 @@ static int ipgre_changelink(struct net_device *dev,
> struct nlattr *tb[],
> struct netlink_ext_ack *extack)
> {
> struct ip_tunnel *t = netdev_priv(dev);
> - struct ip_tunnel_parm p;
> struct ip_tunnel_encap ipencap;
> __u32 fwmark = t->fwmark;
> + struct ip_tunnel_parm p;
> int err;
>
> if (ipgre_netlink_encap_parms(data, &ipencap)) {
> @@ -1322,7 +1346,18 @@ static int ipgre_changelink(struct net_device *dev,
> struct nlattr *tb[],
> err = ipgre_netlink_parms(dev, data, tb, &p, &fwmark);
> if (err < 0)
> return err;
> - return ip_tunnel_changelink(dev, tb, &p, fwmark);
> +
> + err = ip_tunnel_changelink(dev, tb, &p, fwmark);
> + if (err < 0)
> + return err;
> +
> + t->parms.i_flags = p.i_flags;
> + t->parms.o_flags = p.o_flags;
> +
> + if (strcmp(dev->rtnl_link_ops->kind, "erspan"))
> + ipgre_link_update(dev, !tb[IFLA_MTU]);
just to comment: ERSPAN does not need update because its GRE flag is
fixed to SEQ.
Acked-by: William Tu <u9012...@gmail.com>
