Message ID | 20180806180439.16559-2-qiuyu.xiao.qyx@gmail.com |
---|---|
State | Changes Requested |
Headers | show |
Series | IPsec support for tunneling | expand |
Hi Qiuyu, Can you explain a little more about why you need L4 ports for route lookup? Does Linux kernel support different route for different L4 ports? I thought it only uses L3 address for route lookup. On Mon, Aug 6, 2018 at 11:04 AM, Qiuyu Xiao <qiuyu.xiao.qyx@gmail.com> wrote: > This patch adds transport ports information for route lookup so that > IPsec can select geneve tunnel traffic to do encryption. > > Signed-off-by: Qiuyu Xiao <qiuyu.xiao.qyx@gmail.com> > Reviewed-by: Greg Rose <gvrose8192@gmail.com> > Tested-by: Greg Rose <gvrose8192@gmail.com> > --- > datapath/linux/compat/geneve.c | 29 +++++++++++++++++++---------- > 1 file changed, 19 insertions(+), 10 deletions(-) > > diff --git a/datapath/linux/compat/geneve.c b/datapath/linux/compat/ > geneve.c > index 435a23fb7..95a665ddd 100644 > --- a/datapath/linux/compat/geneve.c > +++ b/datapath/linux/compat/geneve.c > @@ -836,7 +836,8 @@ free_dst: > static struct rtable *geneve_get_v4_rt(struct sk_buff *skb, > struct net_device *dev, > struct flowi4 *fl4, > - struct ip_tunnel_info *info) > + struct ip_tunnel_info *info, > + __be16 dport, __be16 sport) > { > bool use_cache = ip_tunnel_dst_cache_usable(skb, info); > struct geneve_dev *geneve = netdev_priv(dev); > @@ -850,6 +851,8 @@ static struct rtable *geneve_get_v4_rt(struct sk_buff > *skb, > memset(fl4, 0, sizeof(*fl4)); > fl4->flowi4_mark = skb->mark; > fl4->flowi4_proto = IPPROTO_UDP; > + fl4->fl4_dport = dport; > + fl4->fl4_sport = sport; the route entry (rt) is from rt = ip_route_output_key(geneve->net, fl4) Does different dport and sport return different route lookup entry? Thanks William > if (info) { > fl4->daddr = info->key.u.ipv4.dst; > @@ -895,7 +898,8 @@ static struct rtable *geneve_get_v4_rt(struct sk_buff > *skb, > static struct dst_entry *geneve_get_v6_dst(struct sk_buff *skb, > struct net_device *dev, > struct flowi6 *fl6, > - struct ip_tunnel_info *info) > + struct ip_tunnel_info *info, > + __be16 dport, __be16 sport) > { > bool use_cache = ip_tunnel_dst_cache_usable(skb, info); > struct geneve_dev *geneve = netdev_priv(dev); > @@ -911,6 +915,8 @@ static struct dst_entry *geneve_get_v6_dst(struct > sk_buff *skb, > memset(fl6, 0, sizeof(*fl6)); > fl6->flowi6_mark = skb->mark; > fl6->flowi6_proto = IPPROTO_UDP; > + fl6->fl6_dport = dport; > + fl6->fl6_sport = sport; > > if (info) { > fl6->daddr = info->key.u.ipv6.dst; > @@ -1005,13 +1011,13 @@ static netdev_tx_t geneve_xmit_skb(struct sk_buff > *skb, struct net_device *dev, > goto tx_error; > } > > - rt = geneve_get_v4_rt(skb, dev, &fl4, info); > + sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true); > + rt = geneve_get_v4_rt(skb, dev, &fl4, info, geneve->dst_port, > sport); > if (IS_ERR(rt)) { > err = PTR_ERR(rt); > goto tx_error; > } > > - sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true); > skb_reset_mac_header(skb); > > iip = ip_hdr(skb); > @@ -1097,13 +1103,13 @@ static netdev_tx_t geneve6_xmit_skb(struct sk_buff > *skb, struct net_device *dev, > } > } > > - dst = geneve_get_v6_dst(skb, dev, &fl6, info); > + sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true); > + dst = geneve_get_v6_dst(skb, dev, &fl6, info, geneve->dst_port, > sport); > if (IS_ERR(dst)) { > err = PTR_ERR(dst); > goto tx_error; > } > > - sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true); > skb_reset_mac_header(skb); > > iip = ip_hdr(skb); > @@ -1232,13 +1238,17 @@ int ovs_geneve_fill_metadata_dst(struct > net_device *dev, struct sk_buff *skb) > struct geneve_dev *geneve = netdev_priv(dev); > struct rtable *rt; > struct flowi4 fl4; > + __be16 sport; > #if IS_ENABLED(CONFIG_IPV6) > struct dst_entry *dst; > struct flowi6 fl6; > #endif > > + sport = udp_flow_src_port(geneve->net, skb, > + 1, USHRT_MAX, true); > + > if (ip_tunnel_info_af(info) == AF_INET) { > - rt = geneve_get_v4_rt(skb, dev, &fl4, info); > + rt = geneve_get_v4_rt(skb, dev, &fl4, info, > geneve->dst_port, sport); > if (IS_ERR(rt)) > return PTR_ERR(rt); > > @@ -1246,7 +1256,7 @@ int ovs_geneve_fill_metadata_dst(struct net_device > *dev, struct sk_buff *skb) > info->key.u.ipv4.src = fl4.saddr; > #if IS_ENABLED(CONFIG_IPV6) > } else if (ip_tunnel_info_af(info) == AF_INET6) { > - dst = geneve_get_v6_dst(skb, dev, &fl6, info); > + dst = geneve_get_v6_dst(skb, dev, &fl6, info, > geneve->dst_port, sport); > if (IS_ERR(dst)) > return PTR_ERR(dst); > > @@ -1257,8 +1267,7 @@ int ovs_geneve_fill_metadata_dst(struct net_device > *dev, struct sk_buff *skb) > return -EINVAL; > } > > - info->key.tp_src = udp_flow_src_port(geneve->net, skb, > - 1, USHRT_MAX, true); > + info->key.tp_src = sport; > info->key.tp_dst = geneve->dst_port; > return 0; > } > -- > 2.18.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
Hi William, ip_route_output_key() calls xfrm_lookup(). xfrm_lookup() needs L4 ports so that the packet can match IPsec's security policy based on L4 ports. IPsec security policy for Geneve selects udp packets with dst port 6081. If no port information, the IPsec stack won't know the packet is a Geneve packet and the packet won't be encrypted. Different dport and sport affect `struct xfrm_state` in the `struct dst_entry`. But this structure only matters to the xfrm module. The Linux upstream VXLAN module already included L4 ports for VXLAN route look up. Thanks, Qiuyu On Thu, Aug 9, 2018 at 2:13 PM, William Tu <u9012063@gmail.com> wrote: > Hi Qiuyu, > > Can you explain a little more about why you need L4 ports for route lookup? > Does Linux kernel support different route for different L4 ports? > I thought it only uses L3 address for route lookup. > > On Mon, Aug 6, 2018 at 11:04 AM, Qiuyu Xiao <qiuyu.xiao.qyx@gmail.com> > wrote: > >> This patch adds transport ports information for route lookup so that >> IPsec can select geneve tunnel traffic to do encryption. >> >> Signed-off-by: Qiuyu Xiao <qiuyu.xiao.qyx@gmail.com> >> Reviewed-by: Greg Rose <gvrose8192@gmail.com> >> Tested-by: Greg Rose <gvrose8192@gmail.com> >> --- >> datapath/linux/compat/geneve.c | 29 +++++++++++++++++++---------- >> 1 file changed, 19 insertions(+), 10 deletions(-) >> >> diff --git a/datapath/linux/compat/geneve.c >> b/datapath/linux/compat/geneve.c >> index 435a23fb7..95a665ddd 100644 >> --- a/datapath/linux/compat/geneve.c >> +++ b/datapath/linux/compat/geneve.c >> @@ -836,7 +836,8 @@ free_dst: >> static struct rtable *geneve_get_v4_rt(struct sk_buff *skb, >> struct net_device *dev, >> struct flowi4 *fl4, >> - struct ip_tunnel_info *info) >> + struct ip_tunnel_info *info, >> + __be16 dport, __be16 sport) >> { >> bool use_cache = ip_tunnel_dst_cache_usable(skb, info); >> struct geneve_dev *geneve = netdev_priv(dev); >> @@ -850,6 +851,8 @@ static struct rtable *geneve_get_v4_rt(struct sk_buff >> *skb, >> memset(fl4, 0, sizeof(*fl4)); >> fl4->flowi4_mark = skb->mark; >> fl4->flowi4_proto = IPPROTO_UDP; >> + fl4->fl4_dport = dport; >> + fl4->fl4_sport = sport; > > > the route entry (rt) is from > rt = ip_route_output_key(geneve->net, fl4) > > Does different dport and sport return different route lookup entry? > > Thanks > William > > >> > if (info) { >> fl4->daddr = info->key.u.ipv4.dst; >> @@ -895,7 +898,8 @@ static struct rtable *geneve_get_v4_rt(struct sk_buff >> *skb, >> static struct dst_entry *geneve_get_v6_dst(struct sk_buff *skb, >> struct net_device *dev, >> struct flowi6 *fl6, >> - struct ip_tunnel_info *info) >> + struct ip_tunnel_info *info, >> + __be16 dport, __be16 sport) >> { >> bool use_cache = ip_tunnel_dst_cache_usable(skb, info); >> struct geneve_dev *geneve = netdev_priv(dev); >> @@ -911,6 +915,8 @@ static struct dst_entry *geneve_get_v6_dst(struct >> sk_buff *skb, >> memset(fl6, 0, sizeof(*fl6)); >> fl6->flowi6_mark = skb->mark; >> fl6->flowi6_proto = IPPROTO_UDP; >> + fl6->fl6_dport = dport; >> + fl6->fl6_sport = sport; >> >> if (info) { >> fl6->daddr = info->key.u.ipv6.dst; >> @@ -1005,13 +1011,13 @@ static netdev_tx_t geneve_xmit_skb(struct sk_buff >> *skb, struct net_device *dev, >> goto tx_error; >> } >> >> - rt = geneve_get_v4_rt(skb, dev, &fl4, info); >> + sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true); >> + rt = geneve_get_v4_rt(skb, dev, &fl4, info, geneve->dst_port, >> sport); >> if (IS_ERR(rt)) { >> err = PTR_ERR(rt); >> goto tx_error; >> } >> >> - sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true); >> skb_reset_mac_header(skb); >> >> iip = ip_hdr(skb); >> @@ -1097,13 +1103,13 @@ static netdev_tx_t geneve6_xmit_skb(struct >> sk_buff *skb, struct net_device *dev, >> } >> } >> >> - dst = geneve_get_v6_dst(skb, dev, &fl6, info); >> + sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true); >> + dst = geneve_get_v6_dst(skb, dev, &fl6, info, geneve->dst_port, >> sport); >> if (IS_ERR(dst)) { >> err = PTR_ERR(dst); >> goto tx_error; >> } >> >> - sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true); >> skb_reset_mac_header(skb); >> >> iip = ip_hdr(skb); >> @@ -1232,13 +1238,17 @@ int ovs_geneve_fill_metadata_dst(struct >> net_device *dev, struct sk_buff *skb) >> struct geneve_dev *geneve = netdev_priv(dev); >> struct rtable *rt; >> struct flowi4 fl4; >> + __be16 sport; >> #if IS_ENABLED(CONFIG_IPV6) >> struct dst_entry *dst; >> struct flowi6 fl6; >> #endif >> >> + sport = udp_flow_src_port(geneve->net, skb, >> + 1, USHRT_MAX, true); >> + >> if (ip_tunnel_info_af(info) == AF_INET) { >> - rt = geneve_get_v4_rt(skb, dev, &fl4, info); >> + rt = geneve_get_v4_rt(skb, dev, &fl4, info, >> geneve->dst_port, sport); >> if (IS_ERR(rt)) >> return PTR_ERR(rt); >> >> @@ -1246,7 +1256,7 @@ int ovs_geneve_fill_metadata_dst(struct net_device >> *dev, struct sk_buff *skb) >> info->key.u.ipv4.src = fl4.saddr; >> #if IS_ENABLED(CONFIG_IPV6) >> } else if (ip_tunnel_info_af(info) == AF_INET6) { >> - dst = geneve_get_v6_dst(skb, dev, &fl6, info); >> + dst = geneve_get_v6_dst(skb, dev, &fl6, info, >> geneve->dst_port, sport); >> if (IS_ERR(dst)) >> return PTR_ERR(dst); >> >> @@ -1257,8 +1267,7 @@ int ovs_geneve_fill_metadata_dst(struct net_device >> *dev, struct sk_buff *skb) >> return -EINVAL; >> } >> >> - info->key.tp_src = udp_flow_src_port(geneve->net, skb, >> - 1, USHRT_MAX, true); >> + info->key.tp_src = sport; >> info->key.tp_dst = geneve->dst_port; >> return 0; >> } >> -- >> 2.18.0 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > >
On Thu, Aug 9, 2018 at 3:28 PM, Qiuyu Xiao <qiuyu.xiao.qyx@gmail.com> wrote: > Hi William, > > ip_route_output_key() calls xfrm_lookup(). xfrm_lookup() needs L4 ports > so that the packet can match IPsec's security policy based on L4 ports. > IPsec security policy for Geneve selects udp packets with dst port 6081. > If no port information, the IPsec stack won't know the packet is a Geneve > packet and the packet won't be encrypted. > > Different dport and sport affect `struct xfrm_state` in the `struct dst_entry`. > But this structure only matters to the xfrm module. The Linux upstream > VXLAN module already included L4 ports for VXLAN route look up. > > I see, thanks! --William
I have one question. In "datapath/linux/compat/include/net/geneve.h", USE_UPSTREAM_TUNNEL decides whether to use Linux upstream kernel function or OVS kernel function to transmit Geneve packet. Currently, it chooses Linux upstream kernel function. How to set USE_UPSTREAM_TUNNEL to use OVS kernel function? Otherwise, even though this patch is applied, IPsec won't work for Geneve tunnel without Linux upstream also being patched? Thanks, Qiuyu On Thu, Aug 9, 2018 at 3:41 PM, William Tu <u9012063@gmail.com> wrote: > > > On Thu, Aug 9, 2018 at 3:28 PM, Qiuyu Xiao <qiuyu.xiao.qyx@gmail.com> > wrote: > >> Hi William, >> >> ip_route_output_key() calls xfrm_lookup(). xfrm_lookup() needs L4 ports >> so that the packet can match IPsec's security policy based on L4 ports. >> IPsec security policy for Geneve selects udp packets with dst port 6081. >> If no port information, the IPsec stack won't know the packet is a >> Geneve packet and the packet won't be encrypted. >> >> Different dport and sport affect `struct xfrm_state` in the `struct dst_entry`. >> But this structure only matters to the xfrm module. The Linux upstream >> VXLAN module already included L4 ports for VXLAN route look up. >> >> > I see, thanks! > > --William >
On Thu, Aug 9, 2018 at 4:13 PM, Qiuyu Xiao <qiuyu.xiao.qyx@gmail.com> wrote: > I have one question. In "datapath/linux/compat/include/net/geneve.h", USE_UPSTREAM_TUNNEL > decides whether to use Linux upstream kernel function or OVS kernel > function to transmit Geneve packet. Currently, it chooses Linux upstream > kernel function. How to set USE_UPSTREAM_TUNNEL to use OVS kernel function? > you can't set USE_UPSTREAM_TUNNEL, it is detected by acinclude by looking at the kernel header files. > Otherwise, even though this patch is applied, IPsec won't work for Geneve > tunnel without Linux upstream also being patched? > another way is to test on different kernel version, for example, 4.8 kernel the USE_UPSTREAM_TUNNEL should be no. --William > > Thanks, > Qiuyu > > On Thu, Aug 9, 2018 at 3:41 PM, William Tu <u9012063@gmail.com> wrote: > >> >> >> On Thu, Aug 9, 2018 at 3:28 PM, Qiuyu Xiao <qiuyu.xiao.qyx@gmail.com> >> wrote: >> >>> Hi William, >>> >>> ip_route_output_key() calls xfrm_lookup(). xfrm_lookup() needs L4 ports >>> so that the packet can match IPsec's security policy based on L4 ports. >>> IPsec security policy for Geneve selects udp packets with dst port >>> 6081. If no port information, the IPsec stack won't know the packet is >>> a Geneve packet and the packet won't be encrypted. >>> >>> Different dport and sport affect `struct xfrm_state` in the `struct dst_entry`. >>> But this structure only matters to the xfrm module. The Linux upstream >>> VXLAN module already included L4 ports for VXLAN route look up. >>> >>> >> I see, thanks! >> >> --William >> > >
diff --git a/datapath/linux/compat/geneve.c b/datapath/linux/compat/geneve.c index 435a23fb7..95a665ddd 100644 --- a/datapath/linux/compat/geneve.c +++ b/datapath/linux/compat/geneve.c @@ -836,7 +836,8 @@ free_dst: static struct rtable *geneve_get_v4_rt(struct sk_buff *skb, struct net_device *dev, struct flowi4 *fl4, - struct ip_tunnel_info *info) + struct ip_tunnel_info *info, + __be16 dport, __be16 sport) { bool use_cache = ip_tunnel_dst_cache_usable(skb, info); struct geneve_dev *geneve = netdev_priv(dev); @@ -850,6 +851,8 @@ static struct rtable *geneve_get_v4_rt(struct sk_buff *skb, memset(fl4, 0, sizeof(*fl4)); fl4->flowi4_mark = skb->mark; fl4->flowi4_proto = IPPROTO_UDP; + fl4->fl4_dport = dport; + fl4->fl4_sport = sport; if (info) { fl4->daddr = info->key.u.ipv4.dst; @@ -895,7 +898,8 @@ static struct rtable *geneve_get_v4_rt(struct sk_buff *skb, static struct dst_entry *geneve_get_v6_dst(struct sk_buff *skb, struct net_device *dev, struct flowi6 *fl6, - struct ip_tunnel_info *info) + struct ip_tunnel_info *info, + __be16 dport, __be16 sport) { bool use_cache = ip_tunnel_dst_cache_usable(skb, info); struct geneve_dev *geneve = netdev_priv(dev); @@ -911,6 +915,8 @@ static struct dst_entry *geneve_get_v6_dst(struct sk_buff *skb, memset(fl6, 0, sizeof(*fl6)); fl6->flowi6_mark = skb->mark; fl6->flowi6_proto = IPPROTO_UDP; + fl6->fl6_dport = dport; + fl6->fl6_sport = sport; if (info) { fl6->daddr = info->key.u.ipv6.dst; @@ -1005,13 +1011,13 @@ static netdev_tx_t geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev, goto tx_error; } - rt = geneve_get_v4_rt(skb, dev, &fl4, info); + sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true); + rt = geneve_get_v4_rt(skb, dev, &fl4, info, geneve->dst_port, sport); if (IS_ERR(rt)) { err = PTR_ERR(rt); goto tx_error; } - sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true); skb_reset_mac_header(skb); iip = ip_hdr(skb); @@ -1097,13 +1103,13 @@ static netdev_tx_t geneve6_xmit_skb(struct sk_buff *skb, struct net_device *dev, } } - dst = geneve_get_v6_dst(skb, dev, &fl6, info); + sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true); + dst = geneve_get_v6_dst(skb, dev, &fl6, info, geneve->dst_port, sport); if (IS_ERR(dst)) { err = PTR_ERR(dst); goto tx_error; } - sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true); skb_reset_mac_header(skb); iip = ip_hdr(skb); @@ -1232,13 +1238,17 @@ int ovs_geneve_fill_metadata_dst(struct net_device *dev, struct sk_buff *skb) struct geneve_dev *geneve = netdev_priv(dev); struct rtable *rt; struct flowi4 fl4; + __be16 sport; #if IS_ENABLED(CONFIG_IPV6) struct dst_entry *dst; struct flowi6 fl6; #endif + sport = udp_flow_src_port(geneve->net, skb, + 1, USHRT_MAX, true); + if (ip_tunnel_info_af(info) == AF_INET) { - rt = geneve_get_v4_rt(skb, dev, &fl4, info); + rt = geneve_get_v4_rt(skb, dev, &fl4, info, geneve->dst_port, sport); if (IS_ERR(rt)) return PTR_ERR(rt); @@ -1246,7 +1256,7 @@ int ovs_geneve_fill_metadata_dst(struct net_device *dev, struct sk_buff *skb) info->key.u.ipv4.src = fl4.saddr; #if IS_ENABLED(CONFIG_IPV6) } else if (ip_tunnel_info_af(info) == AF_INET6) { - dst = geneve_get_v6_dst(skb, dev, &fl6, info); + dst = geneve_get_v6_dst(skb, dev, &fl6, info, geneve->dst_port, sport); if (IS_ERR(dst)) return PTR_ERR(dst); @@ -1257,8 +1267,7 @@ int ovs_geneve_fill_metadata_dst(struct net_device *dev, struct sk_buff *skb) return -EINVAL; } - info->key.tp_src = udp_flow_src_port(geneve->net, skb, - 1, USHRT_MAX, true); + info->key.tp_src = sport; info->key.tp_dst = geneve->dst_port; return 0; }