| Message ID | 1466915256-98925-1-git-send-email-yanhaishuang@cmss.chinamobile.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
> On Jun 26, 2016, at 8:35 PM, zhuyj <zyjzyj2000@gmail.com> wrote: > > + if (geneve->remote.sa.sa_family == AF_INET) > + max_mtu -= sizeof(struct iphdr); > + else > + max_mtu -= sizeof(struct ipv6hdr); > > Sorry, if sa_family is not AF_NET, it is AF_INET6? > > There is a lot of macros in include/linux/socket.h. > > Zhu Yanjun > There are only two enumerations AF_INET and AF_INET6 have been assigned in geneve_newlink: if (data[IFLA_GENEVE_REMOTE] && data[IFLA_GENEVE_REMOTE6]) return -EINVAL; if (data[IFLA_GENEVE_REMOTE]) { remote.sa.sa_family = AF_INET; remote.sin.sin_addr.s_addr = nla_get_in_addr(data[IFLA_GENEVE_REMOTE]); } if (data[IFLA_GENEVE_REMOTE6]) { if (!IS_ENABLED(CONFIG_IPV6)) return -EPFNOSUPPORT; remote.sa.sa_family = AF_INET6; remote.sin6.sin6_addr = nla_get_in6_addr(data[IFLA_GENEVE_REMOTE6]); if (ipv6_addr_type(&remote.sin6.sin6_addr) & IPV6_ADDR_LINKLOCAL) { netdev_dbg(dev, "link-local remote is unsupported\n"); return -EINVAL; } } So I think the else case is for AF_INET6 only.
On Sun, Jun 26, 2016 at 6:13 PM, 严海双 <yanhaishuang@cmss.chinamobile.com> wrote: > >> On Jun 26, 2016, at 8:35 PM, zhuyj <zyjzyj2000@gmail.com> wrote: >> >> + if (geneve->remote.sa.sa_family == AF_INET) >> + max_mtu -= sizeof(struct iphdr); >> + else >> + max_mtu -= sizeof(struct ipv6hdr); >> >> Sorry, if sa_family is not AF_NET, it is AF_INET6? >> >> There is a lot of macros in include/linux/socket.h. >> >> Zhu Yanjun >> > > There are only two enumerations AF_INET and AF_INET6 have been assigned in geneve_newlink: There's actually a third possibility: AF_UNSPEC, which is the default if neither remote type is specified. This is used by lightweight tunnels and should be able to work with either IPv4/v6. For the purposes of the MTU calculation this means that the IPv4 header size should be used to avoid disallowing potentially valid configurations.
On Mon, Jun 27, 2016 at 6:27 PM, 严海双 <yanhaishuang@cmss.chinamobile.com> wrote: > > On Jun 28, 2016, at 12:10 AM, Jesse Gross <jesse@kernel.org> wrote: > > On Sun, Jun 26, 2016 at 6:13 PM, Haishuang Yan > <yanhaishuang@cmss.chinamobile.com> wrote: > > > On Jun 26, 2016, at 8:35 PM, zhuyj <zyjzyj2000@gmail.com> wrote: > > + if (geneve->remote.sa.sa_family == AF_INET) > + max_mtu -= sizeof(struct iphdr); > + else > + max_mtu -= sizeof(struct ipv6hdr); > > Sorry, if sa_family is not AF_NET, it is AF_INET6? > > There is a lot of macros in include/linux/socket.h. > > Zhu Yanjun > > > There are only two enumerations AF_INET and AF_INET6 have been assigned in > geneve_newlink: > > > There's actually a third possibility: AF_UNSPEC, which is the default > if neither remote type is specified. This is used by lightweight > tunnels and should be able to work with either IPv4/v6. For the > purposes of the MTU calculation this means that the IPv4 header size > should be used to avoid disallowing potentially valid configurations. > > > Yes, you’re right. Thanks for you advise. I will send a v2 commit like this: > > if (geneve->remote.sa.sa_family == AF_INET6) > max_mtu -= sizeof(struct ipv6hdr); > else > max_mtu -= sizeof(struct iphdr); > > Is this ok? Yes, that looks fine to me.
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c index aa61708..c676d23 100644 --- a/drivers/net/geneve.c +++ b/drivers/net/geneve.c @@ -1036,12 +1036,17 @@ static netdev_tx_t geneve_xmit(struct sk_buff *skb, struct net_device *dev) static int __geneve_change_mtu(struct net_device *dev, int new_mtu, bool strict) { + struct geneve_dev *geneve = netdev_priv(dev); /* The max_mtu calculation does not take account of GENEVE * options, to avoid excluding potentially valid * configurations. */ - int max_mtu = IP_MAX_MTU - GENEVE_BASE_HLEN - sizeof(struct iphdr) - - dev->hard_header_len; + int max_mtu = IP_MAX_MTU - GENEVE_BASE_HLEN - dev->hard_header_len; + + if (geneve->remote.sa.sa_family == AF_INET) + max_mtu -= sizeof(struct iphdr); + else + max_mtu -= sizeof(struct ipv6hdr); if (new_mtu < 68) return -EINVAL;
For ipv6+udp+geneve encapsulation data, the max_mtu should subtract sizeof(ipv6hdr), instead of sizeof(iphdr). Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com> --- drivers/net/geneve.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)