Re: [ovs-dev] [PATCH v2] ipv6 tunneling: Fix for performance drop introduced by ipv6 tunnel support.
On Thu, Jan 14, 2016 at 01:27:45PM +, Sugesh Chandran wrote:
> Adding a new field called dl_type in flow tunnel structure to verify the
> validity
> of tunnel metadata. This field avoids the need of resetting and validating the
> entire ipv4/ipv6 tunnel destination address which caused a serious performance
> drop.
>
> Fixes: 3ae91c019019 ("tunneling: add IPv6 support to netdev_tunnel_config")
> Signed-off-by: Sugesh Chandran
> ---
> lib/flow.c | 7 ++---
> lib/match.c | 12
> lib/meta-flow.c | 10 +--
> lib/netdev-vport.c | 10 +++
> lib/odp-util.c | 10 +++
> lib/packets.c| 7 -
> lib/packets.h| 69
>
> ofproto/ofproto-dpif-ipfix.c | 3 +-
> ofproto/ofproto-dpif-rid.c | 3 +-
> ofproto/ofproto-dpif-sflow.c | 6 ++--
> ofproto/tunnel.c | 12
> 11 files changed, 96 insertions(+), 53 deletions(-)
>
> diff --git a/lib/flow.c b/lib/flow.c
> index 5668d0c..f9a645b 100644
> --- a/lib/flow.c
> +++ b/lib/flow.c
> @@ -818,15 +818,14 @@ flow_get_metadata(const struct flow *flow, struct match
> *flow_metadata)
> if (flow->tunnel.ip_src) {
> match_set_tun_src(flow_metadata, flow->tunnel.ip_src);
> }
> -if (flow->tunnel.ip_dst) {
> +if (flow->tunnel.dl_type == ETH_TYPE_IP) {
You should use htons(ETH_TYPE_IP) and htons(ETH_TYPE_IPV6) all over the code.
Cascardo.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3] ipv6 tunneling: Fix for performance drop introduced by ipv6 tunnel support.
On Tue, Jan 19, 2016 at 05:23:58PM +, Sugesh Chandran wrote:
> Adding a new field called dl_type in flow tunnel structure to verify the
> validity
> of tunnel metadata. This field avoids the need of resetting and validating the
> entire ipv4/ipv6 tunnel destination address which caused a serious performance
> drop.
>
> Fixes: 3ae91c019019 ("tunneling: add IPv6 support to netdev_tunnel_config")
> Signed-off-by: Sugesh Chandran
Acked-by: Thadeu Lima de Souza Cascardo
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH branch 2.5] tunneling: Disable IPv6 tunnel
On Thu, Feb 04, 2016 at 08:04:53PM -0800, Jesse Gross wrote:
> On Thu, Feb 4, 2016 at 7:55 PM, Pravin B Shelar wrote:
> > There are multiple issues in IPv6 userspace tunnel
> > implementation. There is not enough time to get all
> > fixes in branch-2.5. So it make sense to disable the
> > support on 2.5.
> >
> > Signed-off-by: Pravin B Shelar
>
> This will also affect the kernel implementation - I think that's what
> we want because it's not documented or supported in the kernel module
> that will ship as part of 2.5. However, the commit message isn't quite
> accurate.
>
Is the problem time to code or time to test? I have some of the fixes on my
queue, working on tnl-ports, expect to have it ready today. Will try to have
better tests for those broken pieces as well, but not sure I can get those
before next week.
Cascardo.
> > diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
> > index 88f5022..2234de7 100644
> > --- a/lib/netdev-vport.c
> > +++ b/lib/netdev-vport.c
> > @@ -503,6 +503,10 @@ set_tunnel_config(struct netdev *dev_, const struct
> > smap *args)
> >name, node->value);
> > return EINVAL;
> > }
> > +if (dst_proto == ETH_TYPE_IPV6) {
> > +VLOG_WARN("%s: IPv6 'remote_ip' is not supported", name);
> > +return EOPNOTSUPP;
> > +}
>
> I believe that this won't catch the case where the address is set to
> "flow". I think we probably also need to block attempts to set or
> match IPv6 addresses through OpenFlow.
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/2] packets: Fix UDP over IPv6 checksum
Fix the IPv6 pseudoheader checksum. Requires that the payload length is given to
the function.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/netdev-vport.c | 14 +-
lib/packets.c | 8
lib/packets.h | 2 +-
3 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 88f5022..a2d5093 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -1000,15 +1000,18 @@ udp_extract_tnl_md(struct dp_packet *packet, struct
flow_tnl *tnl,
if (udp->udp_csum) {
uint32_t csum;
+size_t udp_len;
+udp_len = dp_packet_size(packet) -
+ ((const unsigned char *) udp -
+ (const unsigned char *) dp_packet_l2(packet));
if (is_header_ipv6(dp_packet_data(packet))) {
-csum = packet_csum_pseudoheader6(dp_packet_l3(packet));
+csum = packet_csum_pseudoheader6(dp_packet_l3(packet),
+ htonl(udp_len));
} else {
csum = packet_csum_pseudoheader(dp_packet_l3(packet));
}
-csum = csum_continue(csum, udp, dp_packet_size(packet) -
- ((const unsigned char *)udp -
- (const unsigned char *)dp_packet_l2(packet)));
+csum = csum_continue(csum, udp, udp_len);
if (csum_finish(csum)) {
return NULL;
}
@@ -1048,7 +1051,8 @@ push_udp_header(struct dp_packet *packet,
if (udp->udp_csum) {
uint32_t csum;
if (is_header_ipv6(dp_packet_data(packet))) {
-csum = packet_csum_pseudoheader6(ipv6_hdr(dp_packet_data(packet)));
+csum = packet_csum_pseudoheader6(ipv6_hdr(dp_packet_data(packet)),
+ htonl(ip_tot_size));
} else {
csum = packet_csum_pseudoheader(ip_hdr(dp_packet_data(packet)));
}
diff --git a/lib/packets.c b/lib/packets.c
index d82341d..93d75bc 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -1271,7 +1271,8 @@ packet_csum_pseudoheader(const struct ip_header *ip)
#ifndef __CHECKER__
uint32_t
-packet_csum_pseudoheader6(const struct ovs_16aligned_ip6_hdr *ip6)
+packet_csum_pseudoheader6(const struct ovs_16aligned_ip6_hdr *ip6,
+ ovs_be32 plen)
{
uint32_t partial = 0;
@@ -1284,10 +1285,9 @@ packet_csum_pseudoheader6(const struct
ovs_16aligned_ip6_hdr *ip6)
partial = csum_add32(partial, get_16aligned_be32(&(ip6->ip6_dst.be32[2])));
partial = csum_add32(partial, get_16aligned_be32(&(ip6->ip6_dst.be32[3])));
+partial = csum_add32(partial, plen);
partial = csum_add16(partial, 0);
-partial = csum_add16(partial, ip6->ip6_plen);
-partial = csum_add16(partial, 0);
-partial = csum_add16(partial, ip6->ip6_nxt);
+partial = csum_add16(partial, htons(ip6->ip6_nxt));
return partial;
}
diff --git a/lib/packets.h b/lib/packets.h
index f1445de..b8fa8db 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -838,7 +838,7 @@ struct icmp6_header {
};
BUILD_ASSERT_DECL(ICMP6_HEADER_LEN == sizeof(struct icmp6_header));
-uint32_t packet_csum_pseudoheader6(const struct ovs_16aligned_ip6_hdr *);
+uint32_t packet_csum_pseudoheader6(const struct ovs_16aligned_ip6_hdr *,
ovs_be32);
/* Neighbor Discovery option field.
* ND options are always a multiple of 8 bytes in size. */
--
2.5.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 2/2] packets: fix compose_nd to not crash vswitchd
packet_set_ipv6 and packet_set_nd assumed that valid packets were already in
place, so they were not a good fit for composing new packets. In fact,
packet_set_ipv6 didn't even set the IP version or set L4 offset, causing crashes
when compose_nd tried to access L4.
This patch introduces ipv6_compose and uses it for compose_nd, then set the ND
packets without other helpers.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/packets.c | 46 ++
lib/packets.h | 3 +++
2 files changed, 41 insertions(+), 8 deletions(-)
diff --git a/lib/packets.c b/lib/packets.c
index 93d75bc..b794398 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -1226,6 +1226,24 @@ compose_arp(struct dp_packet *b, uint16_t arp_op,
}
void
+ipv6_compose(struct dp_packet *packet, uint8_t proto,
+ const struct in6_addr *src, const struct in6_addr *dst,
+ uint8_t key_tc, ovs_be32 key_fl, uint8_t key_hl)
+{
+struct ovs_16aligned_ip6_hdr *ip6 = dp_packet_l3(packet);
+ovs_be32 old_flow;
+ip6->ip6_vfc = 0x60;
+ip6->ip6_nxt = proto;
+ip6->ip6_hlim = key_hl;
+ip6->ip6_plen = 0;
+old_flow = get_16aligned_be32(&ip6->ip6_flow) & htonl(0xF00F |
~IPV6_LABEL_MASK);
+put_16aligned_be32(&ip6->ip6_flow, old_flow | key_fl | htonl(key_tc <<
20));
+memcpy(&ip6->ip6_src, src, sizeof(ovs_be32[4]));
+memcpy(&ip6->ip6_dst, dst, sizeof(ovs_be32[4]));
+dp_packet_set_l4(packet, ip6 + 1);
+}
+
+void
compose_nd(struct dp_packet *b, const struct eth_addr eth_src,
struct in6_addr * ipv6_src, struct in6_addr * ipv6_dst)
{
@@ -1233,26 +1251,38 @@ compose_nd(struct dp_packet *b, const struct eth_addr
eth_src,
struct eth_addr eth_dst;
struct ovs_nd_msg *ns;
struct ovs_nd_opt *nd_opt;
+struct ovs_16aligned_ip6_hdr *ip6;
+size_t plen = ND_MSG_LEN + ND_OPT_LEN;
+uint32_t csum;
in6_addr_solicited_node(&sn_addr, ipv6_dst);
ipv6_multicast_to_ethernet(ð_dst, &sn_addr);
-eth_compose(b, eth_dst, eth_src, ETH_TYPE_IPV6,
-IPV6_HEADER_LEN + ICMP6_HEADER_LEN + ND_OPT_LEN);
-packet_set_ipv6(b, IPPROTO_ICMPV6,
-ALIGNED_CAST(ovs_be32 *, ipv6_src->s6_addr),
-ALIGNED_CAST(ovs_be32 *, sn_addr.s6_addr),
-0, 0, 255);
+eth_compose(b, eth_dst, eth_src, ETH_TYPE_IPV6, IPV6_HEADER_LEN + plen);
+ipv6_compose(b, IPPROTO_ICMPV6, ipv6_src, &sn_addr, 0, 0, 255);
+ip6 = dp_packet_l3(b);
+ip6->ip6_plen = htons(plen);
ns = dp_packet_l4(b);
+memset(ns, 0, plen);
nd_opt = &ns->options[0];
ns->icmph.icmp6_type = ND_NEIGHBOR_SOLICIT;
ns->icmph.icmp6_code = 0;
+memcpy(ns->target.be32, ipv6_dst, sizeof(ovs_be32[4]));
nd_opt->nd_opt_type = ND_OPT_SOURCE_LINKADDR;
-packet_set_nd(b, ALIGNED_CAST(ovs_be32 *, ipv6_dst->s6_addr),
- eth_src, eth_addr_zero);
+nd_opt->nd_opt_len = 1;
+nd_opt->nd_opt_mac = eth_src;
+
+ns->icmph.icmp6_cksum = 0;
+csum = packet_csum_pseudoheader6(ip6, htonl(plen));
+csum = csum_continue(csum, &ns->icmph, plen);
+csum = csum_finish(csum);
+ns->icmph.icmp6_cksum = csum;
+if (!ns->icmph.icmp6_cksum) {
+ns->icmph.icmp6_cksum = htons(0x);
+}
}
uint32_t
diff --git a/lib/packets.h b/lib/packets.h
index b8fa8db..96a3251 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -1062,6 +1062,9 @@ void packet_set_nd(struct dp_packet *, const ovs_be32
target[4],
void packet_format_tcp_flags(struct ds *, uint16_t);
const char *packet_tcp_flag_to_string(uint32_t flag);
+void ipv6_compose(struct dp_packet *, uint8_t proto, const struct in6_addr
*src,
+ const struct in6_addr *dst, uint8_t tc,
+ ovs_be32 fl, uint8_t hlmit);
void compose_arp(struct dp_packet *, uint16_t arp_op,
const struct eth_addr arp_sha,
const struct eth_addr arp_tha, bool broadcast,
--
2.5.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] tunneling: Disable IPv6 tunnel
On Thu, Feb 11, 2016 at 01:05:16AM -0800, Pravin B Shelar wrote: > There are multiple issues in IPv6 userspace tunnel > implementation. Even the kernel module that ships with > 2.5 does not support IPv6 tunneling. There is not > enough time to get all fixes in branch-2.5. So it make > sense to disable the support on 2.5. > > Signed-off-by: Pravin B Shelar I have plans to test this further, but I agree on pushing this to 2.5. Acked-by: Thadeu Lima de Souza Cascardo ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] netlink-socket: return correct error code when connect fails
When connect and other calls fail after get_socket_rcvbuf, the return code would be the rcvbuf size, not errno from the last call. Signed-off-by: Thadeu Lima de Souza Cascardo --- lib/netlink-socket.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/netlink-socket.c b/lib/netlink-socket.c index 5ef4b15..0cedb41 100644 --- a/lib/netlink-socket.c +++ b/lib/netlink-socket.c @@ -195,6 +195,7 @@ nl_sock_create(int protocol, struct nl_sock **sockp) goto error; } sock->rcvbuf = retval; +retval = 0; /* Connect to kernel (pid 0) as remote address. */ memset(&remote, 0, sizeof remote); -- 2.5.0 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] netdev-linux: fix warning message
Instead of reading
"error receiving Ethernet packet on Permission denied: ens3",
it should read
"error receiving Ethernet packet on ens3: Permission denied".
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/netdev-linux.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 393b4cc..d58c1b1 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -1107,7 +1107,7 @@ netdev_linux_rxq_recv(struct netdev_rxq *rxq_, struct
dp_packet **packets,
if (retval) {
if (retval != EAGAIN && retval != EMSGSIZE) {
VLOG_WARN_RL(&rl, "error receiving Ethernet packet on %s: %s",
- ovs_strerror(errno), netdev_rxq_get_name(rxq_));
+ netdev_rxq_get_name(rxq_), ovs_strerror(errno));
}
dp_packet_delete(buffer);
} else {
--
2.5.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] netdev-linux: fix warning message
On Tue, Feb 16, 2016 at 11:26:11AM +0900, Simon Horman wrote:
> Hi,
>
> On Mon, Feb 15, 2016 at 03:14:21PM -0200, Thadeu Lima de Souza Cascardo wrote:
> > Instead of reading
> >
> > "error receiving Ethernet packet on Permission denied: ens3",
> >
> > it should read
> >
> > "error receiving Ethernet packet on ens3: Permission denied".
>
> It seems to me that the aim of this patch is to make the error message
> format consistent with something else. But I think that if you wish to do
No, not really. It simply doesn't make sense to say you received a packet on an
error message. You received a packet on an interface. I noticed the error by
itself, when running vswitchd on netdev mode with SELinux on enforcing mode,
without all the necessary permissions.
> that you should make all other messages that make use of netdev names or
> strerror, at least within the same source file, consistent.
It's not about consistency, but I agree if there are any other messages like
this, that they should be fixed as well.
Cascardo.
>
> > Signed-off-by: Thadeu Lima de Souza Cascardo
> > ---
> > lib/netdev-linux.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> > index 393b4cc..d58c1b1 100644
> > --- a/lib/netdev-linux.c
> > +++ b/lib/netdev-linux.c
> > @@ -1107,7 +1107,7 @@ netdev_linux_rxq_recv(struct netdev_rxq *rxq_, struct
> > dp_packet **packets,
> > if (retval) {
> > if (retval != EAGAIN && retval != EMSGSIZE) {
> > VLOG_WARN_RL(&rl, "error receiving Ethernet packet on %s: %s",
> > - ovs_strerror(errno), netdev_rxq_get_name(rxq_));
> > + netdev_rxq_get_name(rxq_), ovs_strerror(errno));
> > }
> > dp_packet_delete(buffer);
> > } else {
> > --
> > 2.5.0
> >
> > ___
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] netlink-socket: return correct error code when connect fails
On Tue, Feb 16, 2016 at 11:32:19AM +0900, Simon Horman wrote: > Hi, > > On Mon, Feb 15, 2016 at 03:13:30PM -0200, Thadeu Lima de Souza Cascardo wrote: > > When connect and other calls fail after get_socket_rcvbuf, the return code > > would > > be the rcvbuf size, not errno from the last call. > > > > Signed-off-by: Thadeu Lima de Souza Cascardo > > This seems correct to me and I have pushed it to the master branch. > > Please let me know if you think it is appropriate for backporting > to branch-2.4 or branch-2.5. Thanks, Simon. I noticed the problem on branch-2.5. So, it applies there. I just checked that it should also affect 2.4 and 2.3. Regards. Cascardo. > > Thanks > > > --- > > lib/netlink-socket.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/lib/netlink-socket.c b/lib/netlink-socket.c > > index 5ef4b15..0cedb41 100644 > > --- a/lib/netlink-socket.c > > +++ b/lib/netlink-socket.c > > @@ -195,6 +195,7 @@ nl_sock_create(int protocol, struct nl_sock **sockp) > > goto error; > > } > > sock->rcvbuf = retval; > > +retval = 0; > > > > /* Connect to kernel (pid 0) as remote address. */ > > memset(&remote, 0, sizeof remote); > > -- > > 2.5.0 > > > > ___ > > dev mailing list > > dev@openvswitch.org > > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] ofproto-dpif-xlate: fix crash when using multicast snooping
The revalidator thread may set may_learn and call xlate_actions with no packet
data. If the revalidated flow is IGMPv3 or MLD, vswitchd will crash when trying
to access the NULL packet.
Only process IGMP and MLD flows when there is a packet. This is a similar
behavior than what we have for other special packets.
Not-Signed-off-yet: Thadeu Lima de Souza Cascardo
Reported-by: Yi Ba
Reported-at: http://openvswitch.org/pipermail/discuss/2016-January/020023.html
Fixes: 06994f879c9d ("mcast-snooping: Add Multicast Listener Discovery support")
---
Hi, Yi Ba.
Can you test this patch for your mcast_snooping bug? Remember to enable
OVS_ENABLE_SG_FIREWALL_MULTICAST again. In order to verify it's working, you can
run ovs-vsctl get bridge br-ex mcast_snooping_enable.
Thanks.
Cascardo.
---
AUTHORS | 1 +
ofproto/ofproto-dpif-xlate.c | 4 ++--
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/AUTHORS b/AUTHORS
index 936394d..366b72f 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -412,6 +412,7 @@ Vishal Swarankarvishal.swarn...@gmail.com
Vjekoslav Brajkovic bal...@cs.washington.edu
Voravit T. vora...@kth.se
Yeming Zhao zhaoyem...@gmail.com
+Yi Ba yby.develo...@yahoo.com
Ying Chen yingc...@vmware.com
Yongqiang Liu liuyq7...@gmail.com
ZHANG Zhiming zhangzhim...@yunshan.net.cn
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index a6ea067..7195d45 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2409,7 +2409,7 @@ xlate_normal(struct xlate_ctx *ctx)
if (is_igmp(flow)) {
if (mcast_snooping_is_membership(flow->tp_src) ||
mcast_snooping_is_query(flow->tp_src)) {
-if (ctx->xin->may_learn) {
+if (ctx->xin->may_learn && ctx->xin->packet) {
update_mcast_snooping_table(ctx->xbridge, flow, vlan,
in_xbundle, ctx->xin->packet);
}
@@ -2441,7 +2441,7 @@ xlate_normal(struct xlate_ctx *ctx)
return;
} else if (is_mld(flow)) {
ctx->xout->slow |= SLOW_ACTION;
-if (ctx->xin->may_learn) {
+if (ctx->xin->may_learn && ctx->xin->packet) {
update_mcast_snooping_table(ctx->xbridge, flow, vlan,
in_xbundle, ctx->xin->packet);
}
--
2.5.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: fix crash when using multicast snooping
On Thu, Feb 18, 2016 at 01:35:20PM -0800, Joe Stringer wrote: > On 17 February 2016 at 07:24, Aaron Conole wrote: > > Greetings Thadeu, > > > > Thadeu Lima de Souza Cascardo writes: > > > >> The revalidator thread may set may_learn and call xlate_actions with no > >> packet > >> data. If the revalidated flow is IGMPv3 or MLD, vswitchd will crash when > >> trying > >> to access the NULL packet. > >> > >> Only process IGMP and MLD flows when there is a packet. This is a similar > >> behavior than what we have for other special packets. > > > > Just a question on the approach, would it make sense to check for the > > bundle being null as well? I don't know if the case where packet == NULL > > but bundle != NULL could ever exist so I could be off my rocker. > > If you're referring to "in_xbundle", one of the first things that > xlate_normal() does is to ensure it is non-NULL. > > > Alternately, maybe it's better to just patch the mcast functions to bail > > early (maybe with a ratelimited warning) in this case, so that if bundle > > != NULL but packet == NULL, some of the mcast group LRU cache can be > > updated? Don't know if that makes sense, either (maybe there's a > > security reason not to do that?). > > This codepath shouldn't assume that it has a packet. It is called from > revalidator threads to attribute stats and do side-effects (like > learning, cache refresh). That path doesn't have a copy of a packet to > use for translation. If 'may_learn' is true, it indicates that packets > have hit this flow since the last time stats were attributed, so > learning/cache refresh should occur if applicable. In the case of > special handling of packets which are slowpathed (as indicated by the > "ctx->xout->slow |= SLOW_ACTION" line), it is fine to do nothing when > there is no packet. The slow path handling code will deal with > side-effects. Thanks for the explanation, Joe. I guess this means the patch is good as it is, though it would be nice to have Yi Ba Tested-by. Regards. Cascardo. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [ovs-dev,1/2] packets: Fix UDP over IPv6 checksum
On Mon, Feb 22, 2016 at 04:10:06PM -0800, Ben Pfaff wrote: > On Tue, Feb 09, 2016 at 06:22:04AM -0200, Thadeu Lima de Souza Cascardo wrote: > > Fix the IPv6 pseudoheader checksum. Requires that the payload length is > > given to > > the function. > > > > Signed-off-by: Thadeu Lima de Souza Cascardo > > Thank you for the fix. > > The corresponding function for IPv4 has a test in tests/test-csum.c, in > the function test_pseudo(). Can you please add such a test for IPv6? > > Thanks, > > Ben. Sure, will do for both patches. Thanks. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: fix crash when using multicast snooping
On Mon, Feb 22, 2016 at 04:25:57PM -0800, Ben Pfaff wrote:
> This patch seems obviously correct to me, so whenever you give me a
> Signed-off-by, I'll apply it.
>
> On Wed, Feb 17, 2016 at 12:43:56PM -0200, Thadeu Lima de Souza Cascardo wrote:
> > The revalidator thread may set may_learn and call xlate_actions with no
> > packet
> > data. If the revalidated flow is IGMPv3 or MLD, vswitchd will crash when
> > trying
> > to access the NULL packet.
> >
> > Only process IGMP and MLD flows when there is a packet. This is a similar
> > behavior than what we have for other special packets.
> >
> > Not-Signed-off-yet: Thadeu Lima de Souza Cascardo
> > Reported-by: Yi Ba
> > Reported-at:
> > http://openvswitch.org/pipermail/discuss/2016-January/020023.html
> > Fixes: 06994f879c9d ("mcast-snooping: Add Multicast Listener Discovery
> > support")
We have given enough time for Yi Ba, and given him credit. And I have tested
that it fixes the crash. So,
Signed-off-by: Thadeu Lima de Souza Cascardo
> > ---
> >
> > Hi, Yi Ba.
> >
> > Can you test this patch for your mcast_snooping bug? Remember to enable
> > OVS_ENABLE_SG_FIREWALL_MULTICAST again. In order to verify it's working,
> > you can
> > run ovs-vsctl get bridge br-ex mcast_snooping_enable.
> >
> > Thanks.
> > Cascardo.
> >
> > ---
> > AUTHORS | 1 +
> > ofproto/ofproto-dpif-xlate.c | 4 ++--
> > 2 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/AUTHORS b/AUTHORS
> > index 936394d..366b72f 100644
> > --- a/AUTHORS
> > +++ b/AUTHORS
> > @@ -412,6 +412,7 @@ Vishal Swarankarvishal.swarn...@gmail.com
> > Vjekoslav Brajkovic bal...@cs.washington.edu
> > Voravit T. vora...@kth.se
> > Yeming Zhao zhaoyem...@gmail.com
> > +Yi Ba yby.develo...@yahoo.com
> > Ying Chen yingc...@vmware.com
> > Yongqiang Liu liuyq7...@gmail.com
> > ZHANG Zhiming zhangzhim...@yunshan.net.cn
> > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> > index a6ea067..7195d45 100644
> > --- a/ofproto/ofproto-dpif-xlate.c
> > +++ b/ofproto/ofproto-dpif-xlate.c
> > @@ -2409,7 +2409,7 @@ xlate_normal(struct xlate_ctx *ctx)
> > if (is_igmp(flow)) {
> > if (mcast_snooping_is_membership(flow->tp_src) ||
> > mcast_snooping_is_query(flow->tp_src)) {
> > -if (ctx->xin->may_learn) {
> > +if (ctx->xin->may_learn && ctx->xin->packet) {
> > update_mcast_snooping_table(ctx->xbridge, flow, vlan,
> > in_xbundle,
> > ctx->xin->packet);
> > }
> > @@ -2441,7 +2441,7 @@ xlate_normal(struct xlate_ctx *ctx)
> > return;
> > } else if (is_mld(flow)) {
> > ctx->xout->slow |= SLOW_ACTION;
> > -if (ctx->xin->may_learn) {
> > +if (ctx->xin->may_learn && ctx->xin->packet) {
> > update_mcast_snooping_table(ctx->xbridge, flow, vlan,
> > in_xbundle, ctx->xin->packet);
> > }
> > --
> > 2.5.0
> >
> > ___
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 3/6] lib: Fix compose nd
0, 0, 255,
> + ND_MSG_LEN + ND_OPT_LEN);
>
> ns->icmph.icmp6_type = ND_NEIGHBOR_SOLICIT;
> ns->icmph.icmp6_code = 0;
> +put_16aligned_be32(&ns->rco_flags, htons(0));
>
> +nd_opt = &ns->options[0];
> nd_opt->nd_opt_type = ND_OPT_SOURCE_LINKADDR;
> +nd_opt->nd_opt_len = 1;
> +
> packet_set_nd(b, ALIGNED_CAST(ovs_be32 *, ipv6_dst->s6_addr),
>eth_src, eth_addr_zero);
> +ns->icmph.icmp6_cksum = 0;
> +icmp_csum = packet_csum_pseudoheader6(dp_packet_l3(b));
> +ns->icmph.icmp6_cksum = csum_finish(csum_continue(icmp_csum, ns,
> + ND_MSG_LEN +
> ND_OPT_LEN));
> }
>
> uint32_t
> @@ -1342,15 +1367,14 @@ packet_csum_pseudoheader6(const struct
> ovs_16aligned_ip6_hdr *ip6)
> partial = csum_add32(partial,
> get_16aligned_be32(&(ip6->ip6_src.be32[1])));
> partial = csum_add32(partial,
> get_16aligned_be32(&(ip6->ip6_src.be32[2])));
> partial = csum_add32(partial,
> get_16aligned_be32(&(ip6->ip6_src.be32[3])));
> +
> partial = csum_add32(partial,
> get_16aligned_be32(&(ip6->ip6_dst.be32[0])));
> partial = csum_add32(partial,
> get_16aligned_be32(&(ip6->ip6_dst.be32[1])));
> partial = csum_add32(partial,
> get_16aligned_be32(&(ip6->ip6_dst.be32[2])));
> partial = csum_add32(partial,
> get_16aligned_be32(&(ip6->ip6_dst.be32[3])));
>
> -partial = csum_add16(partial, 0);
> +partial = csum_add16(partial, htons(ip6->ip6_nxt));
> partial = csum_add16(partial, ip6->ip6_plen);
> -partial = csum_add16(partial, 0);
> -partial = csum_add16(partial, ip6->ip6_nxt);
>
> return partial;
> }
> --
> 2.5.0
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
commit e8e8fefe74db26057587bf74eba7d87b0c186449
Author: Thadeu Lima de Souza Cascardo
Date: Fri Mar 4 15:36:50 2016 -0300
test-csum: add IPv6 pseudo header csum test
diff --git a/tests/library.at b/tests/library.at
index d82113f..114bf16 100644
--- a/tests/library.at
+++ b/tests/library.at
@@ -7,7 +7,7 @@ AT_CHECK([ovstest test-flows flows pcap], [0], [checked 247
packets, 0 errors
AT_CLEANUP
AT_SETUP([test TCP/IP checksumming])
-AT_CHECK([ovstest test-csum], [0],
[####
+AT_CHECK([ovstest test-csum], [0],
[#####
])
AT_CLEANUP
diff --git a/tests/test-csum.c b/tests/test-csum.c
index 97638b9..36cdb2b 100644
--- a/tests/test-csum.c
+++ b/tests/test-csum.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -209,6 +210,38 @@ test_pseudo(void)
mark('#');
}
+/* Check the IPv6 pseudoheader calculation. */
+static void
+test_pseudov6(void)
+{
+ovs_be16 csum;
+/* Try an IPv6 header similar to one that the tunnel code
+ * might generate. */
+struct ovs_16aligned_ip6_hdr ip6 = {
+.ip6_vfc = 0x60,
+.ip6_hlim = 64,
+.ip6_nxt = IPPROTO_UDP,
+.ip6_plen = htons(1410),
+.ip6_src = { .be16 = { htons(0x2001), htons(0xcafe), 0, 0, 0, 0, 0,
htons(0x92) } },
+.ip6_dst = { .be16 = { htons(0x2001), htons(0xcafe), 0, 0, 0, 0, 0,
htons(0x91) } }
+};
+
+csum = csum_finish(packet_csum_pseudoheader6(&ip6));
+assert(csum == htons(0x234a));
+
+/* And also test something totally different to check for
+ * corner cases. */
+memset(&ip6, 0xff, sizeof ip6);
+csum = csum_finish(packet_csum_pseudoheader6(&ip6));
+assert(csum == htons(0xff00));
+
+memset(&ip6, 0x00, sizeof ip6);
+csum = csum_finish(packet_csum_pseudoheader6(&ip6));
+assert(csum == htons(0x));
+
+mark('#');
+}
+
static void
test_csum_main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
{
@@ -273,6 +306,7 @@ test_csum_main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
test_rfc1624();
test_crc32c();
test_pseudo();
+test_pseudov6();
/* Test recalc_csum16(). */
for (i = 0; i < 32; i++) {
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] IGMPv3 support
Support IGMPv3 messages with multiple records. Make sure all IGMPv3
messages go through slow path, since they may carry multiple multicast
addresses, unlike IGMPv2.
Tests done:
* multiple addresses in IGMPv3 report are inserted in mdb;
* address is removed from IGMPv3 if record is INCLUDE_MODE;
* reports sent on a burst with same flow all go to userspace;
* IGMPv3 reports go to mrouters, i.e., ports that have issued a query.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/mcast-snooping.c | 52
lib/mcast-snooping.h | 5 +
lib/packets.h| 28
ofproto/ofproto-dpif-xlate.c | 27 +++
4 files changed, 108 insertions(+), 4 deletions(-)
diff --git a/lib/mcast-snooping.c b/lib/mcast-snooping.c
index c3ffd6b..70fada4 100644
--- a/lib/mcast-snooping.c
+++ b/lib/mcast-snooping.c
@@ -69,6 +69,7 @@ mcast_snooping_is_membership(ovs_be16 igmp_type)
switch (ntohs(igmp_type)) {
case IGMP_HOST_MEMBERSHIP_REPORT:
case IGMPV2_HOST_MEMBERSHIP_REPORT:
+case IGMPV3_HOST_MEMBERSHIP_REPORT:
case IGMP_HOST_LEAVE_MESSAGE:
return true;
}
@@ -416,6 +417,57 @@ mcast_snooping_add_group(struct mcast_snooping *ms,
ovs_be32 ip4,
return learned;
}
+int
+mcast_snooping_add_report(struct mcast_snooping *ms,
+ const struct dp_packet *p,
+ uint16_t vlan, void *port)
+{
+ovs_be32 ip4;
+size_t offset;
+const struct igmpv3 *igmpv3;
+const struct igmpv3_record *record;
+int count = 0;
+int ngrp;
+
+offset = p->l4_ofs;
+igmpv3 = dp_packet_at(p, offset, IGMPV3_HDR_LEN);
+if (!igmpv3) {
+return 0;
+}
+ngrp = ntohs(igmpv3->ngrp);
+offset += IGMPV3_HDR_LEN;
+while (ngrp--) {
+bool ret;
+record = dp_packet_at(p, offset, sizeof(struct igmpv3_record));
+if (!record) {
+break;
+}
+/* Only consider known record types. */
+if (record->type < IGMPV3_MODE_IS_INCLUDE
+|| record->type > IGMPV3_BLOCK_OLD_SOURCES) {
+continue;
+}
+ip4 = record->maddr;
+/*
+ * If record is INCLUDE MODE and there are no sources, it's equivalent
+ * to a LEAVE.
+ * */
+if (ntohs(record->nsrcs) == 0
+&& (record->type == IGMPV3_MODE_IS_INCLUDE
+|| record->type == IGMPV3_CHANGE_TO_INCLUDE_MODE)) {
+ret = mcast_snooping_leave_group(ms, ip4, vlan, port);
+} else {
+ret = mcast_snooping_add_group(ms, ip4, vlan, port);
+}
+if (ret) {
+count++;
+}
+offset += sizeof(*record)
+ + ntohs(record->nsrcs) * sizeof(ovs_be32) + record->aux_len;
+}
+return count;
+}
+
bool
mcast_snooping_leave_group(struct mcast_snooping *ms, ovs_be32 ip4,
uint16_t vlan, void *port)
diff --git a/lib/mcast-snooping.h b/lib/mcast-snooping.h
index 979b2aa..f4bc8fb 100644
--- a/lib/mcast-snooping.h
+++ b/lib/mcast-snooping.h
@@ -20,6 +20,7 @@
#define MCAST_SNOOPING_H 1
#include
+#include "dp-packet.h"
#include "hmap.h"
#include "list.h"
#include "ovs-atomic.h"
@@ -181,6 +182,10 @@ mcast_snooping_lookup(const struct mcast_snooping *ms,
ovs_be32 dip,
bool mcast_snooping_add_group(struct mcast_snooping *ms, ovs_be32 ip4,
uint16_t vlan, void *port)
OVS_REQ_WRLOCK(ms->rwlock);
+int mcast_snooping_add_report(struct mcast_snooping *ms,
+ const struct dp_packet *p,
+ uint16_t vlan, void *port)
+OVS_REQ_WRLOCK(ms->rwlock);
bool mcast_snooping_leave_group(struct mcast_snooping *ms, ovs_be32 ip4,
uint16_t vlan, void *port)
OVS_REQ_WRLOCK(ms->rwlock);
diff --git a/lib/packets.h b/lib/packets.h
index b146a50..8c9a679 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -533,12 +533,40 @@ struct igmp_header {
};
BUILD_ASSERT_DECL(IGMP_HEADER_LEN == sizeof(struct igmp_header));
+#define IGMPV3_HEADER_LEN 8
+OVS_PACKED(
+struct igmpv3_header {
+uint8_t type;
+uint8_t rsvr1;
+ovs_be16 csum;
+ovs_be16 rsvr2;
+ovs_be16 ngrp;
+});
+BUILD_ASSERT_DECL(IGMPV3_HEADER_LEN == sizeof(struct igmpv3_header));
+
+#define IGMPV3_RECORD_LEN
+OVS_PACKED(
+struct igmpv3_record {
+uint8_t type;
+uint8_t aux_len;
+ovs_be16 nsrcs;
+ovs_be32 maddr;
+});
+BUILD_ASSERT_DECL(IGMPV3_RECORD_LEN == sizeof(struct igmpv3_record));
+
#define IGMP_HOST_MEMBERSHIP_QUERY0x11 /* From RFC1112 */
#define IGMP_HOST_MEMBERSHIP_REPORT 0x12 /* Ditto */
#define IGMPV2_HOST_MEMBERSHIP_REPORT 0x16 /* V2 version of 0x12 */
#define IGMP_HOST_LEAVE_MESSAGE 0x17
#define IGMPV3_HOST_MEMBERSHIP_REPORT 0x22 /* V3
Re: [ovs-dev] [PATCH] IGMPv3 support
On Fri, Jun 05, 2015 at 10:48:07PM -0700, Ben Pfaff wrote:
> On Wed, Jun 03, 2015 at 04:49:14PM -0300, Thadeu Lima de Souza Cascardo wrote:
> > Support IGMPv3 messages with multiple records. Make sure all IGMPv3
> > messages go through slow path, since they may carry multiple multicast
> > addresses, unlike IGMPv2.
> >
> > Tests done:
> >
> > * multiple addresses in IGMPv3 report are inserted in mdb;
> > * address is removed from IGMPv3 if record is INCLUDE_MODE;
> > * reports sent on a burst with same flow all go to userspace;
> > * IGMPv3 reports go to mrouters, i.e., ports that have issued a query.
> >
> > Signed-off-by: Thadeu Lima de Souza Cascardo
>
> Thanks for working on this!
>
> I get a ton of errors like this trying to compile with this applied:
>
> In file included from ../lib/hmap.h:22:0,
> from ../lib/cfm.h:21,
> from ../ofproto/ofproto-provider.h:36,
> from ../ofproto/bond.h:22,
> from ../ofproto/bond.c:19:
> ../lib/packets.h:555:37: error: expected expression before '==' token
> BUILD_ASSERT_DECL(IGMPV3_RECORD_LEN == sizeof(struct igmpv3_record));
> ^
> ../lib/util.h:48:61: note: in definition of macro 'BUILD_ASSERT__'
> sizeof(struct { unsigned int build_assert_failed : (EXPR) ? 1 : -1;
> })
> ^
> ../lib/packets.h:555:1: note: in expansion of macro 'BUILD_ASSERT_DECL'
> BUILD_ASSERT_DECL(IGMPV3_RECORD_LEN == sizeof(struct igmpv3_record));
> ^
> ../lib/packets.h:555:37: error: bit-field 'build_assert_failed' width not an
> integer constant
> BUILD_ASSERT_DECL(IGMPV3_RECORD_LEN == sizeof(struct igmpv3_record));
> ^
> ../lib/util.h:48:61: note: in definition of macro 'BUILD_ASSERT__'
> sizeof(struct { unsigned int build_assert_failed : (EXPR) ? 1 : -1;
> })
> ^
> ../lib/packets.h:555:1: note: in expansion of macro 'BUILD_ASSERT_DECL'
> BUILD_ASSERT_DECL(IGMPV3_RECORD_LEN == sizeof(struct igmpv3_record));
> ^
Sorry for that. These were some last minute changes. What was I thinking not
testing
those? It must have been all the excitement to get my first patch submitted to
OVS.
I will fix those, test they work just as well as the original, and resend.
Thanks.
Cascardo.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2] IGMPv3 support
Support IGMPv3 messages with multiple records. Make sure all IGMPv3
messages go through slow path, since they may carry multiple multicast
addresses, unlike IGMPv2.
Tests done:
* multiple addresses in IGMPv3 report are inserted in mdb;
* address is removed from IGMPv3 if record is INCLUDE_MODE;
* reports sent on a burst with same flow all go to userspace;
* IGMPv3 reports go to mrouters, i.e., ports that have issued a query.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/mcast-snooping.c | 52
lib/mcast-snooping.h | 5 +
lib/packets.h| 28
ofproto/ofproto-dpif-xlate.c | 27 +++
4 files changed, 108 insertions(+), 4 deletions(-)
diff --git a/lib/mcast-snooping.c b/lib/mcast-snooping.c
index c3ffd6b..6b89a6c 100644
--- a/lib/mcast-snooping.c
+++ b/lib/mcast-snooping.c
@@ -69,6 +69,7 @@ mcast_snooping_is_membership(ovs_be16 igmp_type)
switch (ntohs(igmp_type)) {
case IGMP_HOST_MEMBERSHIP_REPORT:
case IGMPV2_HOST_MEMBERSHIP_REPORT:
+case IGMPV3_HOST_MEMBERSHIP_REPORT:
case IGMP_HOST_LEAVE_MESSAGE:
return true;
}
@@ -416,6 +417,57 @@ mcast_snooping_add_group(struct mcast_snooping *ms,
ovs_be32 ip4,
return learned;
}
+int
+mcast_snooping_add_report(struct mcast_snooping *ms,
+ const struct dp_packet *p,
+ uint16_t vlan, void *port)
+{
+ovs_be32 ip4;
+size_t offset;
+const struct igmpv3_header *igmpv3;
+const struct igmpv3_record *record;
+int count = 0;
+int ngrp;
+
+offset = p->l4_ofs;
+igmpv3 = dp_packet_at(p, offset, IGMPV3_HEADER_LEN);
+if (!igmpv3) {
+return 0;
+}
+ngrp = ntohs(igmpv3->ngrp);
+offset += IGMPV3_HEADER_LEN;
+while (ngrp--) {
+bool ret;
+record = dp_packet_at(p, offset, sizeof(struct igmpv3_record));
+if (!record) {
+break;
+}
+/* Only consider known record types. */
+if (record->type < IGMPV3_MODE_IS_INCLUDE
+|| record->type > IGMPV3_BLOCK_OLD_SOURCES) {
+continue;
+}
+ip4 = record->maddr;
+/*
+ * If record is INCLUDE MODE and there are no sources, it's equivalent
+ * to a LEAVE.
+ * */
+if (ntohs(record->nsrcs) == 0
+&& (record->type == IGMPV3_MODE_IS_INCLUDE
+|| record->type == IGMPV3_CHANGE_TO_INCLUDE_MODE)) {
+ret = mcast_snooping_leave_group(ms, ip4, vlan, port);
+} else {
+ret = mcast_snooping_add_group(ms, ip4, vlan, port);
+}
+if (ret) {
+count++;
+}
+offset += sizeof(*record)
+ + ntohs(record->nsrcs) * sizeof(ovs_be32) + record->aux_len;
+}
+return count;
+}
+
bool
mcast_snooping_leave_group(struct mcast_snooping *ms, ovs_be32 ip4,
uint16_t vlan, void *port)
diff --git a/lib/mcast-snooping.h b/lib/mcast-snooping.h
index 979b2aa..f4bc8fb 100644
--- a/lib/mcast-snooping.h
+++ b/lib/mcast-snooping.h
@@ -20,6 +20,7 @@
#define MCAST_SNOOPING_H 1
#include
+#include "dp-packet.h"
#include "hmap.h"
#include "list.h"
#include "ovs-atomic.h"
@@ -181,6 +182,10 @@ mcast_snooping_lookup(const struct mcast_snooping *ms,
ovs_be32 dip,
bool mcast_snooping_add_group(struct mcast_snooping *ms, ovs_be32 ip4,
uint16_t vlan, void *port)
OVS_REQ_WRLOCK(ms->rwlock);
+int mcast_snooping_add_report(struct mcast_snooping *ms,
+ const struct dp_packet *p,
+ uint16_t vlan, void *port)
+OVS_REQ_WRLOCK(ms->rwlock);
bool mcast_snooping_leave_group(struct mcast_snooping *ms, ovs_be32 ip4,
uint16_t vlan, void *port)
OVS_REQ_WRLOCK(ms->rwlock);
diff --git a/lib/packets.h b/lib/packets.h
index b146a50..c64240c 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -533,12 +533,40 @@ struct igmp_header {
};
BUILD_ASSERT_DECL(IGMP_HEADER_LEN == sizeof(struct igmp_header));
+#define IGMPV3_HEADER_LEN 8
+OVS_PACKED(
+struct igmpv3_header {
+uint8_t type;
+uint8_t rsvr1;
+ovs_be16 csum;
+ovs_be16 rsvr2;
+ovs_be16 ngrp;
+});
+BUILD_ASSERT_DECL(IGMPV3_HEADER_LEN == sizeof(struct igmpv3_header));
+
+#define IGMPV3_RECORD_LEN 8
+OVS_PACKED(
+struct igmpv3_record {
+uint8_t type;
+uint8_t aux_len;
+ovs_be16 nsrcs;
+ovs_be32 maddr;
+});
+BUILD_ASSERT_DECL(IGMPV3_RECORD_LEN == sizeof(struct igmpv3_record));
+
#define IGMP_HOST_MEMBERSHIP_QUERY0x11 /* From RFC1112 */
#define IGMP_HOST_MEMBERSHIP_REPORT 0x12 /* Ditto */
#define IGMPV2_HOST_MEMBERSHIP_REPORT 0x16 /* V2 version of 0x12 */
#define IGMP_HOST_LEAVE_MESSAGE 0x17
#define IGMPV3
Re: [ovs-dev] [PATCH v2] IGMPv3 support
On Tue, Jun 09, 2015 at 03:03:03PM -0300, Flavio Leitner wrote:
> On Mon, Jun 08, 2015 at 01:05:41PM -0300, Thadeu Lima de Souza Cascardo wrote:
> > Support IGMPv3 messages with multiple records. Make sure all IGMPv3
> > messages go through slow path, since they may carry multiple multicast
> > addresses, unlike IGMPv2.
> >
> > Tests done:
> >
> > * multiple addresses in IGMPv3 report are inserted in mdb;
> > * address is removed from IGMPv3 if record is INCLUDE_MODE;
> > * reports sent on a burst with same flow all go to userspace;
> > * IGMPv3 reports go to mrouters, i.e., ports that have issued a query.
> >
> > Signed-off-by: Thadeu Lima de Souza Cascardo
> > ---
> > lib/mcast-snooping.c | 52
> >
> > lib/mcast-snooping.h | 5 +
> > lib/packets.h| 28
> > ofproto/ofproto-dpif-xlate.c | 27 +++
> > 4 files changed, 108 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/mcast-snooping.c b/lib/mcast-snooping.c
> > index c3ffd6b..6b89a6c 100644
> > --- a/lib/mcast-snooping.c
> > +++ b/lib/mcast-snooping.c
> > @@ -69,6 +69,7 @@ mcast_snooping_is_membership(ovs_be16 igmp_type)
> > switch (ntohs(igmp_type)) {
> > case IGMP_HOST_MEMBERSHIP_REPORT:
> > case IGMPV2_HOST_MEMBERSHIP_REPORT:
> > +case IGMPV3_HOST_MEMBERSHIP_REPORT:
> > case IGMP_HOST_LEAVE_MESSAGE:
> > return true;
> > }
> > @@ -416,6 +417,57 @@ mcast_snooping_add_group(struct mcast_snooping *ms,
> > ovs_be32 ip4,
> > return learned;
> > }
> >
> > +int
> > +mcast_snooping_add_report(struct mcast_snooping *ms,
> > + const struct dp_packet *p,
> > + uint16_t vlan, void *port)
> > +{
> > +ovs_be32 ip4;
> > +size_t offset;
> > +const struct igmpv3_header *igmpv3;
> > +const struct igmpv3_record *record;
> > +int count = 0;
> > +int ngrp;
> > +
> > +offset = p->l4_ofs;
>
> The above could be like this:
> offset = dp_packet_l4(p) - dp_packet_data(p)
> to avoid accessing internals of dp_packet.
>
That's how it originally was written, but I thought that using it like
this would protect the code against l4_ofs being UINT16_MAX, and
dp_packet_l4 returning NULL.
Any thoughs on that? Should I just check for dp_packet_l4 returning NULL?
> > +igmpv3 = dp_packet_at(p, offset, IGMPV3_HEADER_LEN);
> > +if (!igmpv3) {
> > +return 0;
> > +}
> > +ngrp = ntohs(igmpv3->ngrp);
> > +offset += IGMPV3_HEADER_LEN;
> > +while (ngrp--) {
> > +bool ret;
> > +record = dp_packet_at(p, offset, sizeof(struct igmpv3_record));
> > +if (!record) {
> > +break;
> > +}
> > +/* Only consider known record types. */
> > +if (record->type < IGMPV3_MODE_IS_INCLUDE
> > +|| record->type > IGMPV3_BLOCK_OLD_SOURCES) {
> > +continue;
> > +}
> > +ip4 = record->maddr;
> > +/*
> > + * If record is INCLUDE MODE and there are no sources, it's
> > equivalent
> > + * to a LEAVE.
> > + * */
> > +if (ntohs(record->nsrcs) == 0
> > +&& (record->type == IGMPV3_MODE_IS_INCLUDE
> > +|| record->type == IGMPV3_CHANGE_TO_INCLUDE_MODE)) {
> > +ret = mcast_snooping_leave_group(ms, ip4, vlan, port);
> > +} else {
> > +ret = mcast_snooping_add_group(ms, ip4, vlan, port);
> > +}
> > +if (ret) {
> > +count++;
> > +}
> > +offset += sizeof(*record)
> > + + ntohs(record->nsrcs) * sizeof(ovs_be32) +
> > record->aux_len;
> > +}
> > +return count;
> > +}
> > +
> > bool
> > mcast_snooping_leave_group(struct mcast_snooping *ms, ovs_be32 ip4,
> > uint16_t vlan, void *port)
> > diff --git a/lib/mcast-snooping.h b/lib/mcast-snooping.h
> > index 979b2aa..f4bc8fb 100644
> > --- a/lib/mcast-snooping.h
> > +++ b/lib/mcast-snooping.h
> > @@ -20,6 +20,7 @@
> > #define MCAST_SNOOPING_H 1
> >
> > #include
> > +#include "dp-packet.h"
> > #include "hmap.h"
> > #include "list.h"
> > #include "ovs-atomic.h"
> &g
Re: [ovs-dev] [PATCH v2] IGMPv3 support
On Thu, Jun 11, 2015 at 12:41:42AM -0300, Flavio Leitner wrote: > > > > + > > > > +offset = p->l4_ofs; > > > > > > The above could be like this: > > > offset = dp_packet_l4(p) - dp_packet_data(p) > > > to avoid accessing internals of dp_packet. > > > > > > > That's how it originally was written, but I thought that using it like > > this would protect the code against l4_ofs being UINT16_MAX, and > > dp_packet_l4 returning NULL. > > > > Any thoughs on that? Should I just check for dp_packet_l4 returning NULL? > > The UINT16_MAX is reserved and means not set. > see dp_packet_reset_offsets() > > [...] I know, but may I just assume that it must have been set since in order to get to this point, a flow has already been extracted from the packet? Cascardo. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] IGMPv3 support
On Tue, Jun 16, 2015 at 08:16:33AM -0700, Ben Pfaff wrote: > On Mon, Jun 08, 2015 at 01:05:41PM -0300, Thadeu Lima de Souza Cascardo wrote: > > Support IGMPv3 messages with multiple records. Make sure all IGMPv3 > > messages go through slow path, since they may carry multiple multicast > > addresses, unlike IGMPv2. > > > > Tests done: > > > > * multiple addresses in IGMPv3 report are inserted in mdb; > > * address is removed from IGMPv3 if record is INCLUDE_MODE; > > * reports sent on a burst with same flow all go to userspace; > > * IGMPv3 reports go to mrouters, i.e., ports that have issued a query. > > > > Signed-off-by: Thadeu Lima de Souza Cascardo > > Thanks! I have a few comments. > > I am surprised that igmpv3_header in packets.h is marked OVS_PACKED, > because it looks like all of the fields are naturally aligned. Does > this structure need to be packed? > > Similarly for igmpv3_record. However, in this struct I would also > change 'maddr' from ovs_be32 to ovs_16aligned_be32, because there are > situations where packet headers can be aligned on an odd 16-bit boundary > and using ovs_16aligned_be32 makes it easier to handle that safely on > RISC architectures. That was me basing my first version of the code on LACP. I was about to test my v3, I will take the chance and apply your suggestions before I test and resubmit. Thanks. Cascardo. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/2] Make IGMP packets always take slow path
IGMP packets need to take the slow path. Otherwise, packets that match
the same flow will not be processed by OVS. That might prevent OVS from
updating the expire time for entries already in the mdb, but also to
lose packets with different addresses in the payload.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
ofproto/ofproto-dpif-xlate.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index f5dc272..0b113a2 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2269,12 +2269,18 @@ xlate_normal(struct xlate_ctx *ctx)
struct mcast_group *grp;
if (flow->nw_proto == IPPROTO_IGMP) {
-if (ctx->xin->may_learn) {
-if (mcast_snooping_is_membership(flow->tp_src) ||
-mcast_snooping_is_query(flow->tp_src)) {
+if (mcast_snooping_is_membership(flow->tp_src) ||
+mcast_snooping_is_query(flow->tp_src)) {
+if (ctx->xin->may_learn) {
update_mcast_snooping_table(ctx->xbridge, flow, vlan,
in_xbundle);
-}
+}
+/*
+ * IGMP packets need to take the slow path, in order to be
+ * processed for mdb updates. That will prevent expires
+* firing off even after hosts have sent reports.
+*/
+ctx->xout->slow |= SLOW_ACTION;
}
if (mcast_snooping_is_membership(flow->tp_src)) {
--
2.4.2
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 2/2] IGMPv3 support
Support IGMPv3 messages with multiple records. Make sure all IGMPv3
messages go through slow path, since they may carry multiple multicast
addresses, unlike IGMPv2.
Tests done:
* multiple addresses in IGMPv3 report are inserted in mdb;
* address is removed from IGMPv3 if record is INCLUDE_MODE;
* reports sent on a burst with same flow all go to userspace;
* IGMPv3 reports go to mrouters, i.e., ports that have issued a query.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/mcast-snooping.c | 52
lib/mcast-snooping.h | 5 +
lib/packets.h| 26 ++
ofproto/ofproto-dpif-xlate.c | 19
4 files changed, 98 insertions(+), 4 deletions(-)
diff --git a/lib/mcast-snooping.c b/lib/mcast-snooping.c
index c3ffd6b..2067a32 100644
--- a/lib/mcast-snooping.c
+++ b/lib/mcast-snooping.c
@@ -69,6 +69,7 @@ mcast_snooping_is_membership(ovs_be16 igmp_type)
switch (ntohs(igmp_type)) {
case IGMP_HOST_MEMBERSHIP_REPORT:
case IGMPV2_HOST_MEMBERSHIP_REPORT:
+case IGMPV3_HOST_MEMBERSHIP_REPORT:
case IGMP_HOST_LEAVE_MESSAGE:
return true;
}
@@ -416,6 +417,57 @@ mcast_snooping_add_group(struct mcast_snooping *ms,
ovs_be32 ip4,
return learned;
}
+int
+mcast_snooping_add_report(struct mcast_snooping *ms,
+ const struct dp_packet *p,
+ uint16_t vlan, void *port)
+{
+ovs_be32 ip4;
+size_t offset;
+const struct igmpv3_header *igmpv3;
+const struct igmpv3_record *record;
+int count = 0;
+int ngrp;
+
+offset = (char *) dp_packet_l4(p) - (char *) dp_packet_data(p);
+igmpv3 = dp_packet_at(p, offset, IGMPV3_HEADER_LEN);
+if (!igmpv3) {
+return 0;
+}
+ngrp = ntohs(igmpv3->ngrp);
+offset += IGMPV3_HEADER_LEN;
+while (ngrp--) {
+bool ret;
+record = dp_packet_at(p, offset, sizeof(struct igmpv3_record));
+if (!record) {
+break;
+}
+/* Only consider known record types. */
+if (record->type < IGMPV3_MODE_IS_INCLUDE
+|| record->type > IGMPV3_BLOCK_OLD_SOURCES) {
+continue;
+}
+ip4 = get_16aligned_be32(&record->maddr);
+/*
+ * If record is INCLUDE MODE and there are no sources, it's equivalent
+ * to a LEAVE.
+ * */
+if (ntohs(record->nsrcs) == 0
+&& (record->type == IGMPV3_MODE_IS_INCLUDE
+|| record->type == IGMPV3_CHANGE_TO_INCLUDE_MODE)) {
+ret = mcast_snooping_leave_group(ms, ip4, vlan, port);
+} else {
+ret = mcast_snooping_add_group(ms, ip4, vlan, port);
+}
+if (ret) {
+count++;
+}
+offset += sizeof(*record)
+ + ntohs(record->nsrcs) * sizeof(ovs_be32) + record->aux_len;
+}
+return count;
+}
+
bool
mcast_snooping_leave_group(struct mcast_snooping *ms, ovs_be32 ip4,
uint16_t vlan, void *port)
diff --git a/lib/mcast-snooping.h b/lib/mcast-snooping.h
index 979b2aa..f4bc8fb 100644
--- a/lib/mcast-snooping.h
+++ b/lib/mcast-snooping.h
@@ -20,6 +20,7 @@
#define MCAST_SNOOPING_H 1
#include
+#include "dp-packet.h"
#include "hmap.h"
#include "list.h"
#include "ovs-atomic.h"
@@ -181,6 +182,10 @@ mcast_snooping_lookup(const struct mcast_snooping *ms,
ovs_be32 dip,
bool mcast_snooping_add_group(struct mcast_snooping *ms, ovs_be32 ip4,
uint16_t vlan, void *port)
OVS_REQ_WRLOCK(ms->rwlock);
+int mcast_snooping_add_report(struct mcast_snooping *ms,
+ const struct dp_packet *p,
+ uint16_t vlan, void *port)
+OVS_REQ_WRLOCK(ms->rwlock);
bool mcast_snooping_leave_group(struct mcast_snooping *ms, ovs_be32 ip4,
uint16_t vlan, void *port)
OVS_REQ_WRLOCK(ms->rwlock);
diff --git a/lib/packets.h b/lib/packets.h
index e22267e..63ad2ff 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -540,12 +540,38 @@ struct igmp_header {
};
BUILD_ASSERT_DECL(IGMP_HEADER_LEN == sizeof(struct igmp_header));
+#define IGMPV3_HEADER_LEN 8
+struct igmpv3_header {
+uint8_t type;
+uint8_t rsvr1;
+ovs_be16 csum;
+ovs_be16 rsvr2;
+ovs_be16 ngrp;
+};
+BUILD_ASSERT_DECL(IGMPV3_HEADER_LEN == sizeof(struct igmpv3_header));
+
+#define IGMPV3_RECORD_LEN 8
+struct igmpv3_record {
+uint8_t type;
+uint8_t aux_len;
+ovs_be16 nsrcs;
+ovs_16aligned_be32 maddr;
+};
+BUILD_ASSERT_DECL(IGMPV3_RECORD_LEN == sizeof(struct igmpv3_record));
+
#define IGMP_HOST_MEMBERSHIP_QUERY0x11 /* From RFC1112 */
#define IGMP_HOST_MEMBERSHIP_REPORT 0x12 /* Ditto */
#define IGMPV2_HOST_MEMBERSHIP_REPORT 0x16 /* V2 version of 0x12 */
#define IGMP_HOST_LEAVE
[ovs-dev] [PATCH v4 2/2] IGMPv3 support
Support IGMPv3 messages with multiple records. Make sure all IGMPv3
messages go through slow path, since they may carry multiple multicast
addresses, unlike IGMPv2.
Tests done:
* multiple addresses in IGMPv3 report are inserted in mdb;
* address is removed from IGMPv3 if record is INCLUDE_MODE;
* reports sent on a burst with same flow all go to userspace;
* IGMPv3 reports go to mrouters, i.e., ports that have issued a query.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/mcast-snooping.c | 52
lib/mcast-snooping.h | 5 +
lib/packets.h| 26 ++
ofproto/ofproto-dpif-xlate.c | 19
4 files changed, 98 insertions(+), 4 deletions(-)
diff --git a/lib/mcast-snooping.c b/lib/mcast-snooping.c
index c3ffd6b..7b927aa 100644
--- a/lib/mcast-snooping.c
+++ b/lib/mcast-snooping.c
@@ -69,6 +69,7 @@ mcast_snooping_is_membership(ovs_be16 igmp_type)
switch (ntohs(igmp_type)) {
case IGMP_HOST_MEMBERSHIP_REPORT:
case IGMPV2_HOST_MEMBERSHIP_REPORT:
+case IGMPV3_HOST_MEMBERSHIP_REPORT:
case IGMP_HOST_LEAVE_MESSAGE:
return true;
}
@@ -416,6 +417,57 @@ mcast_snooping_add_group(struct mcast_snooping *ms,
ovs_be32 ip4,
return learned;
}
+int
+mcast_snooping_add_report(struct mcast_snooping *ms,
+ const struct dp_packet *p,
+ uint16_t vlan, void *port)
+{
+ovs_be32 ip4;
+size_t offset;
+const struct igmpv3_header *igmpv3;
+const struct igmpv3_record *record;
+int count = 0;
+int ngrp;
+
+offset = (char *) dp_packet_l4(p) - (char *) dp_packet_data(p);
+igmpv3 = dp_packet_at(p, offset, IGMPV3_HEADER_LEN);
+if (!igmpv3) {
+return 0;
+}
+ngrp = ntohs(igmpv3->ngrp);
+offset += IGMPV3_HEADER_LEN;
+while (ngrp--) {
+bool ret;
+record = dp_packet_at(p, offset, sizeof(struct igmpv3_record));
+if (!record) {
+break;
+}
+/* Only consider known record types. */
+if (record->type < IGMPV3_MODE_IS_INCLUDE
+|| record->type > IGMPV3_BLOCK_OLD_SOURCES) {
+continue;
+}
+ip4 = get_16aligned_be32(&record->maddr);
+/*
+ * If record is INCLUDE MODE and there are no sources, it's equivalent
+ * to a LEAVE.
+ */
+if (ntohs(record->nsrcs) == 0
+&& (record->type == IGMPV3_MODE_IS_INCLUDE
+|| record->type == IGMPV3_CHANGE_TO_INCLUDE_MODE)) {
+ret = mcast_snooping_leave_group(ms, ip4, vlan, port);
+} else {
+ret = mcast_snooping_add_group(ms, ip4, vlan, port);
+}
+if (ret) {
+count++;
+}
+offset += sizeof(*record)
+ + ntohs(record->nsrcs) * sizeof(ovs_be32) + record->aux_len;
+}
+return count;
+}
+
bool
mcast_snooping_leave_group(struct mcast_snooping *ms, ovs_be32 ip4,
uint16_t vlan, void *port)
diff --git a/lib/mcast-snooping.h b/lib/mcast-snooping.h
index 979b2aa..f4bc8fb 100644
--- a/lib/mcast-snooping.h
+++ b/lib/mcast-snooping.h
@@ -20,6 +20,7 @@
#define MCAST_SNOOPING_H 1
#include
+#include "dp-packet.h"
#include "hmap.h"
#include "list.h"
#include "ovs-atomic.h"
@@ -181,6 +182,10 @@ mcast_snooping_lookup(const struct mcast_snooping *ms,
ovs_be32 dip,
bool mcast_snooping_add_group(struct mcast_snooping *ms, ovs_be32 ip4,
uint16_t vlan, void *port)
OVS_REQ_WRLOCK(ms->rwlock);
+int mcast_snooping_add_report(struct mcast_snooping *ms,
+ const struct dp_packet *p,
+ uint16_t vlan, void *port)
+OVS_REQ_WRLOCK(ms->rwlock);
bool mcast_snooping_leave_group(struct mcast_snooping *ms, ovs_be32 ip4,
uint16_t vlan, void *port)
OVS_REQ_WRLOCK(ms->rwlock);
diff --git a/lib/packets.h b/lib/packets.h
index e22267e..63ad2ff 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -540,12 +540,38 @@ struct igmp_header {
};
BUILD_ASSERT_DECL(IGMP_HEADER_LEN == sizeof(struct igmp_header));
+#define IGMPV3_HEADER_LEN 8
+struct igmpv3_header {
+uint8_t type;
+uint8_t rsvr1;
+ovs_be16 csum;
+ovs_be16 rsvr2;
+ovs_be16 ngrp;
+};
+BUILD_ASSERT_DECL(IGMPV3_HEADER_LEN == sizeof(struct igmpv3_header));
+
+#define IGMPV3_RECORD_LEN 8
+struct igmpv3_record {
+uint8_t type;
+uint8_t aux_len;
+ovs_be16 nsrcs;
+ovs_16aligned_be32 maddr;
+};
+BUILD_ASSERT_DECL(IGMPV3_RECORD_LEN == sizeof(struct igmpv3_record));
+
#define IGMP_HOST_MEMBERSHIP_QUERY0x11 /* From RFC1112 */
#define IGMP_HOST_MEMBERSHIP_REPORT 0x12 /* Ditto */
#define IGMPV2_HOST_MEMBERSHIP_REPORT 0x16 /* V2 version of 0x12 */
#define IGMP_HOST_LEAVE
[ovs-dev] [PATCH v4 1/2] Make IGMP packets always take slow path
IGMP packets need to take the slow path. Otherwise, packets that match
the same flow will not be processed by OVS. That might prevent OVS from
updating the expire time for entries already in the mdb, but also to
lose packets with different addresses in the payload.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
ofproto/ofproto-dpif-xlate.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index f5dc272..6fcf7b8 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2269,12 +2269,18 @@ xlate_normal(struct xlate_ctx *ctx)
struct mcast_group *grp;
if (flow->nw_proto == IPPROTO_IGMP) {
-if (ctx->xin->may_learn) {
-if (mcast_snooping_is_membership(flow->tp_src) ||
-mcast_snooping_is_query(flow->tp_src)) {
+if (mcast_snooping_is_membership(flow->tp_src) ||
+mcast_snooping_is_query(flow->tp_src)) {
+if (ctx->xin->may_learn) {
update_mcast_snooping_table(ctx->xbridge, flow, vlan,
in_xbundle);
-}
+}
+/*
+ * IGMP packets need to take the slow path, in order to be
+ * processed for mdb updates. That will prevent expires
+ * firing off even after hosts have sent reports.
+*/
+ctx->xout->slow |= SLOW_ACTION;
}
if (mcast_snooping_is_membership(flow->tp_src)) {
--
2.4.2
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v4 1/2] Make IGMP packets always take slow path
On Tue, Jun 16, 2015 at 06:25:47PM -0300, Flavio Leitner wrote:
> On Tue, Jun 16, 2015 at 06:01:09PM -0300, Thadeu Lima de Souza Cascardo wrote:
> > IGMP packets need to take the slow path. Otherwise, packets that match
> > the same flow will not be processed by OVS. That might prevent OVS from
> > updating the expire time for entries already in the mdb, but also to
> > lose packets with different addresses in the payload.
> >
> > Signed-off-by: Thadeu Lima de Souza Cascardo
> > ---
> > ofproto/ofproto-dpif-xlate.c | 14 ++
> > 1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> > index f5dc272..6fcf7b8 100644
> > --- a/ofproto/ofproto-dpif-xlate.c
> > +++ b/ofproto/ofproto-dpif-xlate.c
> > @@ -2269,12 +2269,18 @@ xlate_normal(struct xlate_ctx *ctx)
> > struct mcast_group *grp;
> >
> > if (flow->nw_proto == IPPROTO_IGMP) {
> > -if (ctx->xin->may_learn) {
> > -if (mcast_snooping_is_membership(flow->tp_src) ||
> > -mcast_snooping_is_query(flow->tp_src)) {
> > +if (mcast_snooping_is_membership(flow->tp_src) ||
> > +mcast_snooping_is_query(flow->tp_src)) {
> > +if (ctx->xin->may_learn) {
> > update_mcast_snooping_table(ctx->xbridge, flow, vlan,
> > in_xbundle);
> > -}
> > +}
> > +/*
> > + * IGMP packets need to take the slow path, in order to be
> > + * processed for mdb updates. That will prevent expires
> > + * firing off even after hosts have sent reports.
> > +*/
>
> looks like the above isn't correct aligned.
>
> I forgot to tell before about updating the NEWS file:
>
> - Support for multicast snooping (IGMPv1 and IGMPv2
>
> and to update vswitchd/vswitch.xml:
>
> Multicast snooping (RFC 4541) monitors the Internet Group Management
> Protocol (IGMP) traffic between hosts and multicast routers. The
> switch uses what IGMP snooping learns to forward multicast traffic
> only to interfaces that are connected to interested receivers.
> Currently it supports IGMPv1 and IGMPv2 protocols.
>
> fbl
>
Thanks a lot for the review, Flavio.
Excuse my lack of attention to the details. I miss something like
checkpatch on Linux, that would take care of some of the coding style
problems one may find in a patch. Is there any recommendations on that
for OVS?
I have a v5 prepared, but will wait for any other comments before I send
it.
Regards.
Cascardo.
>
> > +ctx->xout->slow |= SLOW_ACTION;
> > }
> >
> > if (mcast_snooping_is_membership(flow->tp_src)) {
> > --
> > 2.4.2
> >
> > ___
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/2] Make IGMP packets always take slow path
IGMP packets need to take the slow path. Otherwise, packets that match
the same flow will not be processed by OVS. That might prevent OVS from
updating the expire time for entries already in the mdb, but also to
lose packets with different addresses in the payload.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
ofproto/ofproto-dpif-xlate.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index f5dc272..a0d13c2 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2269,12 +2269,18 @@ xlate_normal(struct xlate_ctx *ctx)
struct mcast_group *grp;
if (flow->nw_proto == IPPROTO_IGMP) {
-if (ctx->xin->may_learn) {
-if (mcast_snooping_is_membership(flow->tp_src) ||
-mcast_snooping_is_query(flow->tp_src)) {
+if (mcast_snooping_is_membership(flow->tp_src) ||
+mcast_snooping_is_query(flow->tp_src)) {
+if (ctx->xin->may_learn) {
update_mcast_snooping_table(ctx->xbridge, flow, vlan,
in_xbundle);
-}
+}
+/*
+ * IGMP packets need to take the slow path, in order to be
+ * processed for mdb updates. That will prevent expires
+ * firing off even after hosts have sent reports.
+ */
+ctx->xout->slow |= SLOW_ACTION;
}
if (mcast_snooping_is_membership(flow->tp_src)) {
--
2.4.2
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 2/2] IGMPv3 support
Support IGMPv3 messages with multiple records. Make sure all IGMPv3
messages go through slow path, since they may carry multiple multicast
addresses, unlike IGMPv2.
Tests done:
* multiple addresses in IGMPv3 report are inserted in mdb;
* address is removed from IGMPv3 if record is INCLUDE_MODE;
* reports sent on a burst with same flow all go to userspace;
* IGMPv3 reports go to mrouters, i.e., ports that have issued a query.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
NEWS | 2 +-
lib/mcast-snooping.c | 52
lib/mcast-snooping.h | 5 +
lib/packets.h| 26 ++
ofproto/ofproto-dpif-xlate.c | 19
vswitchd/vswitch.xml | 2 +-
6 files changed, 100 insertions(+), 6 deletions(-)
diff --git a/NEWS b/NEWS
index 90d9a29..43461b2 100644
--- a/NEWS
+++ b/NEWS
@@ -87,7 +87,7 @@ Post-v2.3.0
with Docker, the wrapper script will be retired.
- Added support for DPDK Tunneling. VXLAN, GRE, and Geneve are supported
protocols. This is generic tunneling mechanism for userspace datapath.
- - Support for multicast snooping (IGMPv1 and IGMPv2)
+ - Support for multicast snooping (IGMPv1, IGMPv2 and IGMPv3)
- Support for Linux kernels up to 4.0.x
- The documentation now use the term 'destination' to mean one of syslog,
console or file for vlog logging instead of the previously used term
diff --git a/lib/mcast-snooping.c b/lib/mcast-snooping.c
index c3ffd6b..7b927aa 100644
--- a/lib/mcast-snooping.c
+++ b/lib/mcast-snooping.c
@@ -69,6 +69,7 @@ mcast_snooping_is_membership(ovs_be16 igmp_type)
switch (ntohs(igmp_type)) {
case IGMP_HOST_MEMBERSHIP_REPORT:
case IGMPV2_HOST_MEMBERSHIP_REPORT:
+case IGMPV3_HOST_MEMBERSHIP_REPORT:
case IGMP_HOST_LEAVE_MESSAGE:
return true;
}
@@ -416,6 +417,57 @@ mcast_snooping_add_group(struct mcast_snooping *ms,
ovs_be32 ip4,
return learned;
}
+int
+mcast_snooping_add_report(struct mcast_snooping *ms,
+ const struct dp_packet *p,
+ uint16_t vlan, void *port)
+{
+ovs_be32 ip4;
+size_t offset;
+const struct igmpv3_header *igmpv3;
+const struct igmpv3_record *record;
+int count = 0;
+int ngrp;
+
+offset = (char *) dp_packet_l4(p) - (char *) dp_packet_data(p);
+igmpv3 = dp_packet_at(p, offset, IGMPV3_HEADER_LEN);
+if (!igmpv3) {
+return 0;
+}
+ngrp = ntohs(igmpv3->ngrp);
+offset += IGMPV3_HEADER_LEN;
+while (ngrp--) {
+bool ret;
+record = dp_packet_at(p, offset, sizeof(struct igmpv3_record));
+if (!record) {
+break;
+}
+/* Only consider known record types. */
+if (record->type < IGMPV3_MODE_IS_INCLUDE
+|| record->type > IGMPV3_BLOCK_OLD_SOURCES) {
+continue;
+}
+ip4 = get_16aligned_be32(&record->maddr);
+/*
+ * If record is INCLUDE MODE and there are no sources, it's equivalent
+ * to a LEAVE.
+ */
+if (ntohs(record->nsrcs) == 0
+&& (record->type == IGMPV3_MODE_IS_INCLUDE
+|| record->type == IGMPV3_CHANGE_TO_INCLUDE_MODE)) {
+ret = mcast_snooping_leave_group(ms, ip4, vlan, port);
+} else {
+ret = mcast_snooping_add_group(ms, ip4, vlan, port);
+}
+if (ret) {
+count++;
+}
+offset += sizeof(*record)
+ + ntohs(record->nsrcs) * sizeof(ovs_be32) + record->aux_len;
+}
+return count;
+}
+
bool
mcast_snooping_leave_group(struct mcast_snooping *ms, ovs_be32 ip4,
uint16_t vlan, void *port)
diff --git a/lib/mcast-snooping.h b/lib/mcast-snooping.h
index 979b2aa..f4bc8fb 100644
--- a/lib/mcast-snooping.h
+++ b/lib/mcast-snooping.h
@@ -20,6 +20,7 @@
#define MCAST_SNOOPING_H 1
#include
+#include "dp-packet.h"
#include "hmap.h"
#include "list.h"
#include "ovs-atomic.h"
@@ -181,6 +182,10 @@ mcast_snooping_lookup(const struct mcast_snooping *ms,
ovs_be32 dip,
bool mcast_snooping_add_group(struct mcast_snooping *ms, ovs_be32 ip4,
uint16_t vlan, void *port)
OVS_REQ_WRLOCK(ms->rwlock);
+int mcast_snooping_add_report(struct mcast_snooping *ms,
+ const struct dp_packet *p,
+ uint16_t vlan, void *port)
+OVS_REQ_WRLOCK(ms->rwlock);
bool mcast_snooping_leave_group(struct mcast_snooping *ms, ovs_be32 ip4,
uint16_t vlan, void *port)
OVS_REQ_WRLOCK(ms->rwlock);
diff --git a/lib/packets.h b/lib/packets.h
index e22267e..63ad2ff 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -540,12 +540,38 @@ struct igmp_header {
};
BUILD_ASSERT_DECL(IGMP_HEADER
[ovs-dev] [PATCH 2/3] mcast-snooping: Use IPv6 address for MDB
Use IPv6 internally for storing multicast addresses. IPv4 addresses are
translated to their IPv4-mapped equivalent.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/mcast-snooping.c | 69 +++-
lib/mcast-snooping.h | 24 +++
ofproto/ofproto-dpif-xlate.c | 6 ++--
ofproto/ofproto-dpif.c | 5 ++--
4 files changed, 79 insertions(+), 25 deletions(-)
diff --git a/lib/mcast-snooping.c b/lib/mcast-snooping.c
index 7b927aa..f2684f3 100644
--- a/lib/mcast-snooping.c
+++ b/lib/mcast-snooping.c
@@ -87,10 +87,11 @@ mcast_bundle_age(const struct mcast_snooping *ms,
}
static uint32_t
-mcast_table_hash(const struct mcast_snooping *ms, ovs_be32 grp_ip4,
- uint16_t vlan)
+mcast_table_hash(const struct mcast_snooping *ms,
+ const struct in6_addr *grp_addr, uint16_t vlan)
{
-return hash_3words((OVS_FORCE uint32_t) grp_ip4, vlan, ms->secret);
+return hash_words((const uint32_t *) grp_addr->s6_addr, 4,
+ hash_2words(ms->secret, vlan));
}
static struct mcast_group_bundle *
@@ -108,8 +109,8 @@ mcast_group_from_lru_node(struct ovs_list *list)
/* Searches 'ms' for and returns an mcast group for destination address
* 'dip' in 'vlan'. */
struct mcast_group *
-mcast_snooping_lookup(const struct mcast_snooping *ms, ovs_be32 dip,
- uint16_t vlan)
+mcast_snooping_lookup(const struct mcast_snooping *ms,
+ const struct in6_addr *dip, uint16_t vlan)
OVS_REQ_RDLOCK(ms->rwlock)
{
struct mcast_group *grp;
@@ -117,13 +118,32 @@ mcast_snooping_lookup(const struct mcast_snooping *ms,
ovs_be32 dip,
hash = mcast_table_hash(ms, dip, vlan);
HMAP_FOR_EACH_WITH_HASH (grp, hmap_node, hash, &ms->table) {
-if (grp->vlan == vlan && grp->ip4 == dip) {
+if (grp->vlan == vlan && ipv6_addr_equals(&grp->addr, dip)) {
return grp;
}
}
return NULL;
}
+static inline void
+in6_addr_set_mapped_ipv4(struct in6_addr *addr, ovs_be32 ip4)
+{
+union ovs_16aligned_in6_addr *taddr = (void *) addr;
+memset(taddr->be16, 0, sizeof(taddr->be16));
+taddr->be16[5] = 0x;
+put_16aligned_be32(&taddr->be32[3], ip4);
+}
+
+struct mcast_group *
+mcast_snooping_lookup4(const struct mcast_snooping *ms, ovs_be32 ip4,
+ uint16_t vlan)
+OVS_REQ_RDLOCK(ms->rwlock)
+{
+struct in6_addr addr;
+in6_addr_set_mapped_ipv4(&addr, ip4);
+return mcast_snooping_lookup(ms, &addr, vlan);
+}
+
/* If the LRU list is not empty, stores the least-recently-used entry
* in '*e' and returns true. Otherwise, if the LRU list is empty,
* stores NULL in '*e' and return false. */
@@ -376,7 +396,8 @@ mcast_snooping_prune_expired(struct mcast_snooping *ms,
* move to the last position in the LRU list.
*/
bool
-mcast_snooping_add_group(struct mcast_snooping *ms, ovs_be32 ip4,
+mcast_snooping_add_group(struct mcast_snooping *ms,
+ const struct in6_addr *addr,
uint16_t vlan, void *port)
OVS_REQ_WRLOCK(ms->rwlock)
{
@@ -390,9 +411,9 @@ mcast_snooping_add_group(struct mcast_snooping *ms,
ovs_be32 ip4,
}
learned = false;
-grp = mcast_snooping_lookup(ms, ip4, vlan);
+grp = mcast_snooping_lookup(ms, addr, vlan);
if (!grp) {
-uint32_t hash = mcast_table_hash(ms, ip4, vlan);
+uint32_t hash = mcast_table_hash(ms, addr, vlan);
if (hmap_count(&ms->table) >= ms->max_entries) {
group_get_lru(ms, &grp);
@@ -401,7 +422,7 @@ mcast_snooping_add_group(struct mcast_snooping *ms,
ovs_be32 ip4,
grp = xmalloc(sizeof *grp);
hmap_insert(&ms->table, &grp->hmap_node, hash);
-grp->ip4 = ip4;
+memcpy(grp->addr.s6_addr, addr->s6_addr, sizeof(addr->s6_addr));
grp->vlan = vlan;
list_init(&grp->bundle_lru);
learned = true;
@@ -417,6 +438,16 @@ mcast_snooping_add_group(struct mcast_snooping *ms,
ovs_be32 ip4,
return learned;
}
+bool
+mcast_snooping_add_group4(struct mcast_snooping *ms, ovs_be32 ip4,
+ uint16_t vlan, void *port)
+OVS_REQ_WRLOCK(ms->rwlock)
+{
+struct in6_addr addr;
+in6_addr_set_mapped_ipv4(&addr, ip4);
+return mcast_snooping_add_group(ms, &addr, vlan, port);
+}
+
int
mcast_snooping_add_report(struct mcast_snooping *ms,
const struct dp_packet *p,
@@ -455,9 +486,9 @@ mcast_snooping_add_report(struct mcast_snooping *ms,
if (ntohs(record->nsrcs) == 0
&& (record->type == IGMPV3_MODE_IS_INCLUDE
|| record->type == IGMPV3_CHANGE_TO_INCLUDE_MODE)) {
-ret = mcast_snooping_leave_group(ms, ip4, v
[ovs-dev] [PATCH 1/3] ofproto/ofproto-dpif.c: fix coding style
Identation was one extra level at ofproto_unixctl_mcast_snooping_show. Signed-off-by: Thadeu Lima de Souza Cascardo --- ofproto/ofproto-dpif.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 05a80b7..04f6229 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -4442,7 +4442,7 @@ ofproto_unixctl_mcast_snooping_show(struct unixctl_conn *conn, bundle = mrouter->port; ofputil_port_to_string(ofbundle_get_a_port(bundle)->up.ofp_port, name, sizeof name); -ds_put_format(&ds, "%5s %4d querier %3d\n", +ds_put_format(&ds, "%5s %4d querier %3d\n", name, mrouter->vlan, mcast_mrouter_age(ofproto->ms, mrouter)); } -- 2.4.2 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 3/3] Multicast Listener Discovery support
Add support for MLDv1 and MLDv2. The behavior is not that different from
IGMP. Packets to all-hosts address and queries are always flooded,
reports go to routers, routers are added when a query is observed, and
all MLD packets go through slow path.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/flow.h | 25 +
lib/mcast-snooping.c | 67 ++
lib/mcast-snooping.h | 4 +++
lib/packets.c| 1 +
lib/packets.h| 40 +
ofproto/ofproto-dpif-xlate.c | 85 +---
6 files changed, 209 insertions(+), 13 deletions(-)
diff --git a/lib/flow.h b/lib/flow.h
index 70554e4..e68dd74 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -758,6 +758,31 @@ static inline bool is_icmpv6(const struct flow *flow)
&& flow->nw_proto == IPPROTO_ICMPV6);
}
+static inline bool is_igmp(const struct flow *flow)
+{
+return (flow->dl_type == htons(ETH_TYPE_IP)
+&& flow->nw_proto == IPPROTO_IGMP);
+}
+
+static inline bool is_mld(const struct flow *flow)
+{
+return is_icmpv6(flow)
+ && (flow->tp_src == htons(MLD_QUERY)
+ || flow->tp_src == htons(MLD_REPORT)
+ || flow->tp_src == htons(MLD_DONE)
+ || flow->tp_src == htons(MLD2_REPORT));
+}
+
+static inline bool is_mld_query(const struct flow *flow)
+{
+return is_icmpv6(flow) && flow->tp_src == htons(MLD_QUERY);
+}
+
+static inline bool is_mld_report(const struct flow *flow)
+{
+return is_mld(flow) && !is_mld_query(flow);
+}
+
static inline bool is_stp(const struct flow *flow)
{
return (eth_addr_equals(flow->dl_dst, eth_addr_stp)
diff --git a/lib/mcast-snooping.c b/lib/mcast-snooping.c
index f2684f3..aee80b1 100644
--- a/lib/mcast-snooping.c
+++ b/lib/mcast-snooping.c
@@ -499,6 +499,73 @@ mcast_snooping_add_report(struct mcast_snooping *ms,
return count;
}
+int
+mcast_snooping_add_mld(struct mcast_snooping *ms,
+ const struct dp_packet *p,
+ uint16_t vlan, void *port)
+{
+const struct in6_addr *addr;
+size_t offset;
+const struct mld_header *mld;
+const struct mld2_record *record;
+int count = 0;
+int ngrp;
+bool ret;
+
+offset = (char *) dp_packet_l4(p) - (char *) dp_packet_data(p);
+mld = dp_packet_at(p, offset, MLD_HEADER_LEN);
+if (!mld) {
+return 0;
+}
+ngrp = ntohs(mld->ngrp);
+offset += MLD_HEADER_LEN;
+addr = dp_packet_at(p, offset, sizeof(struct in6_addr));
+
+switch (mld->type) {
+case MLD_REPORT:
+ret = mcast_snooping_add_group(ms, addr, vlan, port);
+if (ret)
+count++;
+break;
+case MLD_DONE:
+ret = mcast_snooping_leave_group(ms, addr, vlan, port);
+if (ret)
+count++;
+break;
+case MLD2_REPORT:
+while (ngrp--) {
+record = dp_packet_at(p, offset, sizeof(struct mld2_record));
+if (!record) {
+break;
+}
+/* Only consider known record types. */
+if (record->type < IGMPV3_MODE_IS_INCLUDE
+|| record->type > IGMPV3_BLOCK_OLD_SOURCES) {
+continue;
+}
+addr = &record->maddr;
+/*
+ * If record is INCLUDE MODE and there are no sources, it's
equivalent
+ * to a LEAVE.
+ */
+if (ntohs(record->nsrcs) == 0
+&& (record->type == IGMPV3_MODE_IS_INCLUDE
+|| record->type == IGMPV3_CHANGE_TO_INCLUDE_MODE)) {
+ret = mcast_snooping_leave_group(ms, addr, vlan, port);
+} else {
+ret = mcast_snooping_add_group(ms, addr, vlan, port);
+}
+if (ret) {
+count++;
+}
+offset += sizeof(*record)
+ + ntohs(record->nsrcs) * sizeof(struct in6_addr) +
record->aux_len;
+}
+}
+
+return count;
+}
+
bool
mcast_snooping_leave_group(struct mcast_snooping *ms,
const struct in6_addr *addr,
diff --git a/lib/mcast-snooping.h b/lib/mcast-snooping.h
index e3d15e4..99c314d 100644
--- a/lib/mcast-snooping.h
+++ b/lib/mcast-snooping.h
@@ -194,6 +194,10 @@ int mcast_snooping_add_report(struct mcast_snooping *ms,
const struct dp_packet *p,
uint16_t vlan, void *port)
OVS_REQ_WRLOCK(ms->rwlock);
+int mcast_snooping_add_mld(struct mcast_snooping *ms,
+ const struct dp_packet *p,
+ uint16_t vlan, void *port)
+OVS_REQ_WRLOCK(ms->rwlock);
bool mcast_snooping_leave_group(struct mcast_snooping *ms,
[ovs-dev] [PATCH v2 0/2] Multicast Listener Discovery support
This series introduces Multicast Listener Discovery support. It converts internal representation of addresses to IPv6, including mapping IPv4 to mapped IPv4. Thadeu Lima de Souza Cascardo (2): mcast-snooping: Use IPv6 address for MDB Multicast Listener Discovery support NEWS | 1 + lib/flow.h | 25 lib/mcast-snooping.c | 140 ++- lib/mcast-snooping.h | 28 +++-- lib/packets.c| 12 lib/packets.h| 41 + ofproto/ofproto-dpif-xlate.c | 90 +++- ofproto/ofproto-dpif.c | 5 +- vswitchd/vswitch.xml | 9 +-- 9 files changed, 310 insertions(+), 41 deletions(-) -- 2.4.2 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2 2/2] Multicast Listener Discovery support
Add support for MLDv1 and MLDv2. The behavior is not that different from
IGMP. Packets to all-hosts address and queries are always flooded,
reports go to routers, routers are added when a query is observed, and
all MLD packets go through slow path.
v2:
* Avoid an infinite loop when an unknown record type is found.
* Remove ip4 parameter from update_mcast_snooping_table4__ to make it symmetric
to update_mcast_snooping_table6__.
* Use ovs_16aligned_in6_addr in mld2_record to avoid misalignment.
* Ignore queries from any addresses, not the other way around.
* Address coding style.
* Update vswitch.xml and NEWS.
Signed-off-by: Thadeu Lima de Souza Cascardo
Cc: Flavio Leitner
Cc: Ben Pfaff
---
NEWS | 1 +
lib/flow.h | 25 +
lib/mcast-snooping.c | 71
lib/mcast-snooping.h | 4 +++
lib/packets.c| 1 +
lib/packets.h| 40 +
ofproto/ofproto-dpif-xlate.c | 86 +---
vswitchd/vswitch.xml | 9 ++---
8 files changed, 220 insertions(+), 17 deletions(-)
diff --git a/NEWS b/NEWS
index c59f3a8..d6db7d0 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,7 @@ Post-v2.4.0
* Group chaining (where one OpenFlow group triggers another) is
now supported.
- Support for matching and generating options with Geneve tunnels.
+ - Support Multicast Listener Discovery (MLDv1 and MLDv2).
v2.4.0 - xx xxx
diff --git a/lib/flow.h b/lib/flow.h
index 384a031..a1c6e97 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -758,6 +758,31 @@ static inline bool is_icmpv6(const struct flow *flow)
&& flow->nw_proto == IPPROTO_ICMPV6);
}
+static inline bool is_igmp(const struct flow *flow)
+{
+return (flow->dl_type == htons(ETH_TYPE_IP)
+&& flow->nw_proto == IPPROTO_IGMP);
+}
+
+static inline bool is_mld(const struct flow *flow)
+{
+return is_icmpv6(flow)
+ && (flow->tp_src == htons(MLD_QUERY)
+ || flow->tp_src == htons(MLD_REPORT)
+ || flow->tp_src == htons(MLD_DONE)
+ || flow->tp_src == htons(MLD2_REPORT));
+}
+
+static inline bool is_mld_query(const struct flow *flow)
+{
+return is_icmpv6(flow) && flow->tp_src == htons(MLD_QUERY);
+}
+
+static inline bool is_mld_report(const struct flow *flow)
+{
+return is_mld(flow) && !is_mld_query(flow);
+}
+
static inline bool is_stp(const struct flow *flow)
{
return (eth_addr_equals(flow->dl_dst, eth_addr_stp)
diff --git a/lib/mcast-snooping.c b/lib/mcast-snooping.c
index 39463fa..ba4141e 100644
--- a/lib/mcast-snooping.c
+++ b/lib/mcast-snooping.c
@@ -499,6 +499,77 @@ mcast_snooping_add_report(struct mcast_snooping *ms,
return count;
}
+int
+mcast_snooping_add_mld(struct mcast_snooping *ms,
+ const struct dp_packet *p,
+ uint16_t vlan, void *port)
+{
+const struct in6_addr *addr;
+size_t offset;
+const struct mld_header *mld;
+const struct mld2_record *record;
+int count = 0;
+int ngrp;
+bool ret;
+
+offset = (char *) dp_packet_l4(p) - (char *) dp_packet_data(p);
+mld = dp_packet_at(p, offset, MLD_HEADER_LEN);
+if (!mld) {
+return 0;
+}
+ngrp = ntohs(mld->ngrp);
+offset += MLD_HEADER_LEN;
+addr = dp_packet_at(p, offset, sizeof(struct in6_addr));
+
+switch (mld->type) {
+case MLD_REPORT:
+ret = mcast_snooping_add_group(ms, addr, vlan, port);
+if (ret) {
+count++;
+}
+break;
+case MLD_DONE:
+ret = mcast_snooping_leave_group(ms, addr, vlan, port);
+if (ret) {
+count++;
+}
+break;
+case MLD2_REPORT:
+while (ngrp--) {
+record = dp_packet_at(p, offset, sizeof(struct mld2_record));
+if (!record) {
+break;
+}
+/* Only consider known record types. */
+if (record->type >= IGMPV3_MODE_IS_INCLUDE
+&& record->type <= IGMPV3_BLOCK_OLD_SOURCES) {
+struct in6_addr maddr;
+memcpy(maddr.s6_addr, record->maddr.be16, 16);
+addr = &maddr;
+/*
+ * If record is INCLUDE MODE and there are no sources, it's
+ * equivalent to a LEAVE.
+ */
+if (record->nsrcs == htons(0)
+&& (record->type == IGMPV3_MODE_IS_INCLUDE
+|| record->type == IGMPV3_CHANGE_TO_INCLUDE_MODE)) {
+ret = mcast_snooping_leave_group(ms, addr, vlan, port);
+} else {
+ret = mcast_snooping_add_group(ms, addr, vlan, port);
+}
+if (ret) {
+
[ovs-dev] [PATCH v2 1/2] mcast-snooping: Use IPv6 address for MDB
Use IPv6 internally for storing multicast addresses. IPv4 addresses are
translated to their IPv4-mapped equivalent.
v2:
* Print mapped IPv4 addresses like IPv4 addresses.
* Fix alignment and other issues pointed out by clang, sparse and Ben Pfaff.
Signed-off-by: Thadeu Lima de Souza Cascardo
Cc: Flavio Leitner
Cc: Ben Pfaff
---
lib/mcast-snooping.c | 69 +++-
lib/mcast-snooping.h | 24 +++
lib/packets.c| 11 +++
lib/packets.h| 1 +
ofproto/ofproto-dpif-xlate.c | 6 ++--
ofproto/ofproto-dpif.c | 5 ++--
6 files changed, 91 insertions(+), 25 deletions(-)
diff --git a/lib/mcast-snooping.c b/lib/mcast-snooping.c
index 7b927aa..39463fa 100644
--- a/lib/mcast-snooping.c
+++ b/lib/mcast-snooping.c
@@ -87,10 +87,11 @@ mcast_bundle_age(const struct mcast_snooping *ms,
}
static uint32_t
-mcast_table_hash(const struct mcast_snooping *ms, ovs_be32 grp_ip4,
- uint16_t vlan)
+mcast_table_hash(const struct mcast_snooping *ms,
+ const struct in6_addr *grp_addr, uint16_t vlan)
{
-return hash_3words((OVS_FORCE uint32_t) grp_ip4, vlan, ms->secret);
+return hash_bytes(grp_addr->s6_addr, 16,
+ hash_2words(ms->secret, vlan));
}
static struct mcast_group_bundle *
@@ -108,8 +109,8 @@ mcast_group_from_lru_node(struct ovs_list *list)
/* Searches 'ms' for and returns an mcast group for destination address
* 'dip' in 'vlan'. */
struct mcast_group *
-mcast_snooping_lookup(const struct mcast_snooping *ms, ovs_be32 dip,
- uint16_t vlan)
+mcast_snooping_lookup(const struct mcast_snooping *ms,
+ const struct in6_addr *dip, uint16_t vlan)
OVS_REQ_RDLOCK(ms->rwlock)
{
struct mcast_group *grp;
@@ -117,13 +118,32 @@ mcast_snooping_lookup(const struct mcast_snooping *ms,
ovs_be32 dip,
hash = mcast_table_hash(ms, dip, vlan);
HMAP_FOR_EACH_WITH_HASH (grp, hmap_node, hash, &ms->table) {
-if (grp->vlan == vlan && grp->ip4 == dip) {
+if (grp->vlan == vlan && ipv6_addr_equals(&grp->addr, dip)) {
return grp;
}
}
return NULL;
}
+static inline void
+in6_addr_set_mapped_ipv4(struct in6_addr *addr, ovs_be32 ip4)
+{
+union ovs_16aligned_in6_addr *taddr = (void *) addr;
+memset(taddr->be16, 0, sizeof(taddr->be16));
+taddr->be16[5] = OVS_BE16_MAX;
+put_16aligned_be32(&taddr->be32[3], ip4);
+}
+
+struct mcast_group *
+mcast_snooping_lookup4(const struct mcast_snooping *ms, ovs_be32 ip4,
+ uint16_t vlan)
+OVS_REQ_RDLOCK(ms->rwlock)
+{
+struct in6_addr addr;
+in6_addr_set_mapped_ipv4(&addr, ip4);
+return mcast_snooping_lookup(ms, &addr, vlan);
+}
+
/* If the LRU list is not empty, stores the least-recently-used entry
* in '*e' and returns true. Otherwise, if the LRU list is empty,
* stores NULL in '*e' and return false. */
@@ -376,7 +396,8 @@ mcast_snooping_prune_expired(struct mcast_snooping *ms,
* move to the last position in the LRU list.
*/
bool
-mcast_snooping_add_group(struct mcast_snooping *ms, ovs_be32 ip4,
+mcast_snooping_add_group(struct mcast_snooping *ms,
+ const struct in6_addr *addr,
uint16_t vlan, void *port)
OVS_REQ_WRLOCK(ms->rwlock)
{
@@ -390,9 +411,9 @@ mcast_snooping_add_group(struct mcast_snooping *ms,
ovs_be32 ip4,
}
learned = false;
-grp = mcast_snooping_lookup(ms, ip4, vlan);
+grp = mcast_snooping_lookup(ms, addr, vlan);
if (!grp) {
-uint32_t hash = mcast_table_hash(ms, ip4, vlan);
+uint32_t hash = mcast_table_hash(ms, addr, vlan);
if (hmap_count(&ms->table) >= ms->max_entries) {
group_get_lru(ms, &grp);
@@ -401,7 +422,7 @@ mcast_snooping_add_group(struct mcast_snooping *ms,
ovs_be32 ip4,
grp = xmalloc(sizeof *grp);
hmap_insert(&ms->table, &grp->hmap_node, hash);
-grp->ip4 = ip4;
+grp->addr = *addr;
grp->vlan = vlan;
list_init(&grp->bundle_lru);
learned = true;
@@ -417,6 +438,16 @@ mcast_snooping_add_group(struct mcast_snooping *ms,
ovs_be32 ip4,
return learned;
}
+bool
+mcast_snooping_add_group4(struct mcast_snooping *ms, ovs_be32 ip4,
+ uint16_t vlan, void *port)
+OVS_REQ_WRLOCK(ms->rwlock)
+{
+struct in6_addr addr;
+in6_addr_set_mapped_ipv4(&addr, ip4);
+return mcast_snooping_add_group(ms, &addr, vlan, port);
+}
+
int
mcast_snooping_add_report(struct mcast_snooping *ms,
const struct dp_packet *p,
@@ -455,9 +486,9 @@ mcast_snooping_add_report(struct mcast_snooping *ms,
if (ntohs(record->nsrcs) == 0
[ovs-dev] [PATCH] netdev-linux: do not warn when getting stats for netdev with no vport
When there is no vport for a given netdev, dpif_netlink_vport_get might return
ENODEV. Do not warn a failure to get port stats when that's the case.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/netdev-linux.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 36e27e0..0656f36 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -1597,7 +1597,7 @@ get_stats_via_vport(const struct netdev *netdev_,
int error;
error = get_stats_via_vport__(netdev_, stats);
-if (error && error != ENOENT) {
+if (error && error != ENOENT && error != ENODEV) {
VLOG_WARN_RL(&rl, "%s: obtaining netdev stats via vport failed "
"(%s)",
netdev_get_name(netdev_), ovs_strerror(error));
--
2.4.2
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] netdev-linux: do not warn when getting stats for netdev with no vport
On Mon, Jul 13, 2015 at 09:02:17AM -0700, Ben Pfaff wrote: > On Tue, Jul 07, 2015 at 03:38:51PM -0300, Thadeu Lima de Souza Cascardo wrote: > > When there is no vport for a given netdev, dpif_netlink_vport_get might > > return > > ENODEV. Do not warn a failure to get port stats when that's the case. > > > > Signed-off-by: Thadeu Lima de Souza Cascardo > > Can you give some more context about when this gets reported? I haven't > seen anyone complaining about spurious warnings. Just use a bridge with type netdev, it will show up right after starting vswitchd, for the internal port and linux ports. Since they are not added to the netlink dpif, trying to get vports stats will return ENODEV. Feel free to paraphrase that on the commit message, or ask me to send a v2. Thanks a lot. Cascardo. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Support for MCAST_Snooping
On Sat, Jul 18, 2015 at 02:46:40PM +0530, laxmikanta.beh...@gmail.com wrote:
> Hi,
>
> I am currently using the beta version of ovs(2.3.90) and have enabled the
> multicast snooping configuration with the help of below command.
>
>
> echo 2 >
> /proc/sys/net/ipv4/conf/all/force_igmp_version
> Configure bridge \fBbr0\fR to enable multicast snooping
> ovs-vsctl set Bridge br0
> mcast_snooping_enable=true
> Set the multicast snooping aging time \fBbr0\fR to 300 seconds
> ovs-vsctl set Bridge br0
> other_config:mcast-snooping-aging-time=300
> Set the multicast snooping table size \fBbr0\fR to 2048 entries
> ovs-vsctl set Bridge br0
> other_config:mcast-snooping-table-size=2048
> Disable flooding of unregistered multicast packets to all ports
> ovs-vsctl set Bridge br0
> other_config:mcast-snooping-disable-flood-unregistered=true
> Enable flooding of multicast packets on a specific port
> ovs-vsctl set Port enp9s0f0
> other_config:mcast-snooping-flood=true
>
> Flow added
> [root@Iron utilities]# ovs-ofctl add-flow br0 action=NORMAL
>
> Then I verify the bridge detail and multicast table detail using below
> command and I got below out put however Mcast table is showing empty.
>
> [root@Iron utilities]# ovs-vsctl list bridge
> _uuid : 55e136ae-abe0-4b0e-a99d-c96ecd4cc74e
> auto_attach : []
> controller : []
> datapath_id : "a0369f4f3894"
> datapath_type : ""
> datapath_version: ""
> external_ids: {}
> fail_mode : []
> flood_vlans : []
> flow_tables : {}
> ipfix : []
> mcast_snooping_enable: true
> mirrors : []
> name: "br0"
> netflow : []
> other_config: {mcast-snooping-aging-time="300",
> mcast-snooping-table-size="2048"}
> ports : [121608b8-9dfa-440e-a8f0-33a4112135e4,
> 4f8bb613-fb2e-4c28-b85a-a706e85e1910, bb71c085-a88f-4ad4-a3e4-e228473dfaeb]
> protocols : []
> rstp_enable : false
> rstp_status : {}
> sflow : []
> status : {}
> stp_enable : false
>
> [root@Iron utilities]# ./ovs-appctl fdb/show br0
> port VLAN MACAge
> 3 0 a0:36:9f:4f:38:95 0
>
> [root@Iron utilities]# ovs-appctl mdb/show br0
> port VLAN GROUPAge
>
> Can you please tell me if I missed out any configuration, if not why
> multicast table showing empty???/ Please any one can help me on this.
>
>
> Can you confirm as to whether the multicast snooping configuration is
> supported
> at the group level? If yes then can you mention as to where this information
> (registered group addresses and the mac addresses) is maintained on the
> switch.
>
> As far as i can see the host receives multicast traffic from all the
> multicast groups irrespective of the group it has registered for. Is there
> any other further configuration that is required?
>
What is your bridge topology? Can you send ovs-vsctl show? Does your host belong
to any group, ie, has it sent any joins? Do you have a single port in the
bridge? And are you receiving joins on that port, or only multicast messages
without any joins?
Multicast snooping will not prevent local multicast (224.0.0.0/24) messages from
flooding. And it will not prevent multicast messages from getting into the ports
in any way. It will just control how those messages are forwarded to the other
ports on the bridge.
For example, if you have a group of 5 VMs, all attached to a port each to the
bridge, and three of them join a non-local group (say 239.255.255.1), messages
to that group will only be forwarded to those three VMs.
By the way, I wouldn't suggest you force IGMP version 2. If there is a querier
on your network, it might use a different version and it would be preferred to
use that version. Ideally, it would be using IGMPv3, which uses less
messages/datagrams.
Hope that helps.
Cascardo.
>
>
>
> Thanks,
> Laxmikanta
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Support for MCAST_Snooping
On Sat, Jul 18, 2015 at 06:44:20PM +0530, laxmikanta.beh...@gmail.com wrote: > Hi Cascardo, > > Appreciated your quick response and find my below inline comment for > resolving the issue . > > What is your bridge topology? > Laxmikanta:-My topology is very simple back to back connection. > >Query--> > <--Join > TG1(As a Server) > OVS2.3.9---TG2(As a Host). > > > Can you send ovs-vsctl show? > Laxmikanta :- > > [root@Iron vswitchd]# ovs-vsctl show > > 6d16c545-2a2a-46bf-bfee-be3dc5f408e8 > > Bridge "br0" > > Port "enp9s0f0" > > Interface "enp9s0f0" > > Port "enp9s0f3" > > Interface "enp9s0f3" > > Port "br0" > > Interface "br0" > > type: internal > > Does your host belong to any group, ie, has it sent any joins? > Laxmikanta:- Yes, TG2 sending join message. > > Do you have a single port in the bridge? > Laxmikanta:- Two port in the bridge. > > And are you receiving joins on that port, or only multicast messages > without any joins? > Laxmikanta:- I am able to see the multicast message and query message in > wirashark. > Can you share that dump containing query/join and multicast message? It would be nice to know the MAC addresses of ports br0, enp9s0f3, enp9s0f0 and other MAC addresses belonging to TG1 and TG2, so we can try to match them with the addresses in the dump. Cascardo. > > Thanks & Regards, > > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] perf-counter: fix segfaults on non-linux platforms
Commit 0eee08dbddea520536197657da7a0b (perf-counter: simplify the
performance macro) introduces the PERF macro, which is empty for non-linux
platforms.
Added to commit bc487aeff2d6823c088d6e4499e0f53e6651523b (ovsdb: Add per
transaction commit instruction counter), that uses such macro, it causes
segfaults, since it makes ovsdb_txn_commit return an uninitialized pointer as
error.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/perf-counter.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/perf-counter.h b/lib/perf-counter.h
index ca3aaba..e13c9eb 100644
--- a/lib/perf-counter.h
+++ b/lib/perf-counter.h
@@ -113,7 +113,7 @@ char *perf_counters_to_string(void);
perf_counter_accumulate(&c, start_count); \
}
#else
-#define PERF(name, expr)
+#define PERF(name, expr) expr;
static inline void perf_counters_init(void) {}
static inline void perf_counters_destroy(void) {}
--
2.4.2
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 2/3] rtbsd: support RTM_IFANNOUNCE messages
When devices are created, they are announced using RTM_IFANNOUNCE messages using
PF_ROUTE.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/rtbsd.c | 24 +---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/lib/rtbsd.c b/lib/rtbsd.c
index 8fc88e4..5f58f5a 100644
--- a/lib/rtbsd.c
+++ b/lib/rtbsd.c
@@ -124,8 +124,15 @@ rtbsd_notifier_run(void)
if (retval >= 0) {
/* received packet from PF_ROUTE socket
* XXX check for bad packets */
-if (msg.ifm_type == RTM_IFINFO) {
+switch (msg.ifm_type) {
+case RTM_IFINFO:
+/* Since RTM_IFANNOUNCE messages are smaller than RTM_IFINFO
+ * messages, the same buffer may be used. */
+case RTM_IFANNOUNCE:
rtbsd_report_change(&msg);
+break;
+default:
+break;
}
} else if (errno == EAGAIN) {
ovs_mutex_unlock(&rtbsd_mutex);
@@ -161,14 +168,25 @@ rtbsd_report_change(const struct if_msghdr *msg)
{
struct rtbsd_notifier *notifier;
struct rtbsd_change change;
+const struct if_announcemsghdr *ahdr;
COVERAGE_INC(rtbsd_changed);
change.msg_type = msg->ifm_type; //XXX
-change.if_index = msg->ifm_index;
-if_indextoname(msg->ifm_index, change.if_name);
change.master_ifindex = 0; //XXX
+switch (msg->ifm_type) {
+case RTM_IFINFO:
+change.if_index = msg->ifm_index;
+if_indextoname(msg->ifm_index, change.if_name);
+break;
+case RTM_IFANNOUNCE:
+ahdr = msg;
+change.if_index = ahdr->ifan_index;
+strncpy(change.if_name, ahdr->ifan_name, IF_NAMESIZE);
+break;
+}
+
LIST_FOR_EACH (notifier, node, &all_notifiers) {
notifier->cb(&change, notifier->aux);
}
--
2.4.2
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 3/3] bridge: reconfigure when system interfaces change
Whenever system interfaces are removed, added or change state, reconfigure
bridge. This allows late interfaces to be added to the datapath when they are
added to the system after ovs-vswitchd is started.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/automake.mk | 3 +++
lib/if-notifier-bsd.c | 68 +++
lib/if-notifier.c | 64
lib/if-notifier.h | 32
vswitchd/bridge.c | 24 +-
5 files changed, 190 insertions(+), 1 deletion(-)
create mode 100644 lib/if-notifier-bsd.c
create mode 100644 lib/if-notifier.c
create mode 100644 lib/if-notifier.h
diff --git a/lib/automake.mk b/lib/automake.mk
index 018e62c..4946d90 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -338,6 +338,8 @@ if LINUX
lib_libopenvswitch_la_SOURCES += \
lib/dpif-netlink.c \
lib/dpif-netlink.h \
+ lib/if-notifier.c \
+ lib/if-notifier.h \
lib/netdev-linux.c \
lib/netdev-linux.h \
lib/netlink-notifier.c \
@@ -384,6 +386,7 @@ endif
if HAVE_IF_DL
lib_libopenvswitch_la_SOURCES += \
+ lib/if-notifier-bsd.c \
lib/netdev-bsd.c \
lib/rtbsd.c \
lib/rtbsd.h \
diff --git a/lib/if-notifier-bsd.c b/lib/if-notifier-bsd.c
new file mode 100644
index 000..06874b0
--- /dev/null
+++ b/lib/if-notifier-bsd.c
@@ -0,0 +1,68 @@
+/*
+ * Copyright (c) 2014 Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include
+#include "if-notifier.h"
+#include "rtbsd.h"
+#include "util.h"
+
+struct if_notifier {
+struct rtbsd_notifier notifier;
+if_notify_func *cb;
+void *aux;
+};
+
+static void if_notifier_cb(const struct rtbsd_change *change, void *aux)
+{
+struct if_notifier *notifier;
+notifier = aux;
+notifier->cb(notifier->aux);
+}
+
+struct if_notifier *
+if_notifier_create(if_notify_func *cb, void *aux)
+{
+struct if_notifier *notifier;
+int ret;
+notifier = xzalloc(sizeof *notifier);
+notifier->cb = cb;
+notifier->aux = aux;
+ret = rtbsd_notifier_register(¬ifier->notifier, if_notifier_cb,
+ notifier);
+if (ret) {
+free(notifier);
+return NULL;
+}
+return notifier;
+}
+
+void if_notifier_destroy(struct if_notifier *notifier)
+{
+if (notifier) {
+rtbsd_notifier_unregister(¬ifier->notifier);
+free(notifier);
+}
+}
+
+void if_notifier_run(void)
+{
+rtbsd_notifier_run();
+}
+
+void if_notifier_wait(void)
+{
+rtbsd_notifier_wait();
+}
diff --git a/lib/if-notifier.c b/lib/if-notifier.c
new file mode 100644
index 000..1bf77c5
--- /dev/null
+++ b/lib/if-notifier.c
@@ -0,0 +1,64 @@
+/*
+ * Copyright (c) 2014 Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include
+#include "if-notifier.h"
+#include "rtnetlink-link.h"
+#include "util.h"
+
+struct if_notifier {
+struct nln_notifier *notifier;
+if_notify_func *cb;
+void *aux;
+};
+
+static
+void if_notifier_cb(const struct rtnetlink_link_change *change OVS_UNUSED,
+ void *aux)
+{
+struct if_notifier *notifier;
+notifier = aux;
+notifier->cb(notifier->aux);
+}
+
+struct if_notifier *
+if_notifier_create(if_notify_func *cb, void *aux)
+{
+struct if_notifier *notifier;
+notifier = xmalloc(sizeof *notifier);
+notifier->cb = cb;
+notifier->aux = aux;
+notifier->notifier = rtnetlink_link_notifier_create(if_notifier_cb,
notifier);
+return notifier;
+}
+
+void if_notifier_destroy(struct if_notifier *notifier)
+{
+if (notifier) {
+rtnetlink_link_notifier_destroy(notifier->notifier);
+free(notifier);
+}
+}
+
+void if_notifier_run(void)
+
[ovs-dev] [PATCH 1/3] rtbsd: include necessary headers on rtbsd.h
rtbsd.h requires net/if.h for IF_NAMESIZE. This was not caught because rtbsd.c did not include rtbsd.h as its first header. Signed-off-by: Thadeu Lima de Souza Cascardo --- lib/rtbsd.c | 3 ++- lib/rtbsd.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/rtbsd.c b/lib/rtbsd.c index c3dbdca..8fc88e4 100644 --- a/lib/rtbsd.c +++ b/lib/rtbsd.c @@ -16,6 +16,8 @@ #include +#include "rtbsd.h" + #include #include #include @@ -27,7 +29,6 @@ #include "socket-util.h" #include "poll-loop.h" #include "openvswitch/vlog.h" -#include "rtbsd.h" VLOG_DEFINE_THIS_MODULE(rtbsd); COVERAGE_DEFINE(rtbsd_changed); diff --git a/lib/rtbsd.h b/lib/rtbsd.h index 5dfbaff..d6b79f3 100644 --- a/lib/rtbsd.h +++ b/lib/rtbsd.h @@ -17,6 +17,7 @@ #ifndef RTBSD_H #define RTBSD_H 1 +#include #include "list.h" /* -- 2.4.2 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 3/3] bridge: reconfigure when system interfaces change
On Mon, Jul 27, 2015 at 03:59:18PM -0700, Ben Pfaff wrote: > On Mon, Jul 27, 2015 at 03:55:52PM -0700, Gurucharan Shetty wrote: > > I wonder what happens for Windows here. > > > I guess we need an if-notifier-windows.c also. > Sorry for that, I forgot to include a stub for the other cases. That would include ESX, right? I am just following how the route-table works. Cascardo. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/3] rtbsd: support RTM_IFANNOUNCE messages
On Mon, Jul 27, 2015 at 03:46:28PM -0700, Ben Pfaff wrote: > On Mon, Jul 27, 2015 at 02:24:19PM -0300, Thadeu Lima de Souza Cascardo wrote: > > When devices are created, they are announced using RTM_IFANNOUNCE messages > > using > > PF_ROUTE. > > > > Signed-off-by: Thadeu Lima de Souza Cascardo > > Do you know anyone who might be qualified to properly review this? It > seems reasonable but I don't know this part of BSD at all. Maybe one of the guys who contributed some of the specific BSD code: Ed Maste contributed lib/route-table-bsd.c, and Giuseppe Lettieri contributed lib/rtbsd.c. I am cc'ing them on this message, and bouncing the patch to them as well. What about the style? Is it OK to read using struct if_msghdr, then cast to struct if_announcemsghdr? From my tests, this works fine, the RAW socket seems to work like a DGRAM socket in this particular case. Cascardo. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/3] rtbsd: support RTM_IFANNOUNCE messages
On Tue, Jul 28, 2015 at 10:22:47AM -0700, Ben Pfaff wrote: > On Tue, Jul 28, 2015 at 09:48:56AM -0300, Thadeu Lima de Souza Cascardo wrote: > > On Mon, Jul 27, 2015 at 03:46:28PM -0700, Ben Pfaff wrote: > > > On Mon, Jul 27, 2015 at 02:24:19PM -0300, Thadeu Lima de Souza Cascardo > > > wrote: > > > > When devices are created, they are announced using RTM_IFANNOUNCE > > > > messages using > > > > PF_ROUTE. > > > > > > > > Signed-off-by: Thadeu Lima de Souza Cascardo > > > > > > Do you know anyone who might be qualified to properly review this? It > > > seems reasonable but I don't know this part of BSD at all. > > > > Maybe one of the guys who contributed some of the specific BSD code: > > > > Ed Maste contributed lib/route-table-bsd.c, and > > Giuseppe Lettieri contributed lib/rtbsd.c. > > > > I am cc'ing them on this message, and bouncing the patch to them as well. > > > > What about the style? Is it OK to read using struct if_msghdr, then cast to > > struct if_announcemsghdr? From my tests, this works fine, the RAW socket > > seems > > to work like a DGRAM socket in this particular case. > > Doesn't the compiler complain about that without a cast? Good point. I thought there was a cast already there. Fixed. I was pointing out to the assumption that one message is larger than the other. I don't think this is a problem in this particular case. if_msghdr has a struct at the end, which stores a lot of stats, and all the other earlier members of both if_msghdr and if_announcemsghdr are of types that don't have different sizes on different supported architectures, AFAIK. I know almost nothing about *BSD. Cascardo. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/2] rtbsd: support RTM_IFANNOUNCE messages
When devices are created, they are announced using RTM_IFANNOUNCE messages using
PF_ROUTE.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/rtbsd.c | 24 +---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/lib/rtbsd.c b/lib/rtbsd.c
index 8fc88e4..33fb9fd 100644
--- a/lib/rtbsd.c
+++ b/lib/rtbsd.c
@@ -124,8 +124,15 @@ rtbsd_notifier_run(void)
if (retval >= 0) {
/* received packet from PF_ROUTE socket
* XXX check for bad packets */
-if (msg.ifm_type == RTM_IFINFO) {
+switch (msg.ifm_type) {
+case RTM_IFINFO:
+/* Since RTM_IFANNOUNCE messages are smaller than RTM_IFINFO
+ * messages, the same buffer may be used. */
+case RTM_IFANNOUNCE:
rtbsd_report_change(&msg);
+break;
+default:
+break;
}
} else if (errno == EAGAIN) {
ovs_mutex_unlock(&rtbsd_mutex);
@@ -161,14 +168,25 @@ rtbsd_report_change(const struct if_msghdr *msg)
{
struct rtbsd_notifier *notifier;
struct rtbsd_change change;
+const struct if_announcemsghdr *ahdr;
COVERAGE_INC(rtbsd_changed);
change.msg_type = msg->ifm_type; //XXX
-change.if_index = msg->ifm_index;
-if_indextoname(msg->ifm_index, change.if_name);
change.master_ifindex = 0; //XXX
+switch (msg->ifm_type) {
+case RTM_IFINFO:
+change.if_index = msg->ifm_index;
+if_indextoname(msg->ifm_index, change.if_name);
+break;
+case RTM_IFANNOUNCE:
+ahdr = (const struct if_announcemsghdr *) msg;
+change.if_index = ahdr->ifan_index;
+strncpy(change.if_name, ahdr->ifan_name, IF_NAMESIZE);
+break;
+}
+
LIST_FOR_EACH (notifier, node, &all_notifiers) {
notifier->cb(&change, notifier->aux);
}
--
2.4.2
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 2/2] bridge: reconfigure when system interfaces change
Whenever system interfaces are removed, added or change state, reconfigure
bridge. This allows late interfaces to be added to the datapath when they are
added to the system after ovs-vswitchd is started.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/automake.mk| 7 -
lib/if-notifier-bsd.c | 69 ++
lib/if-notifier-stub.c | 38 +++
lib/if-notifier.c | 64 ++
lib/if-notifier.h | 32 +++
vswitchd/bridge.c | 24 +-
6 files changed, 232 insertions(+), 2 deletions(-)
create mode 100644 lib/if-notifier-bsd.c
create mode 100644 lib/if-notifier-stub.c
create mode 100644 lib/if-notifier.c
create mode 100644 lib/if-notifier.h
diff --git a/lib/automake.mk b/lib/automake.mk
index faca968..a7e7b9b 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -292,6 +292,7 @@ lib_libopenvswitch_la_SOURCES += \
lib/getrusage-windows.c \
lib/latch-windows.c \
lib/route-table-stub.c \
+ lib/if-notifier-stub.c \
lib/strsep.c
else
lib_libopenvswitch_la_SOURCES += \
@@ -338,6 +339,8 @@ if LINUX
lib_libopenvswitch_la_SOURCES += \
lib/dpif-netlink.c \
lib/dpif-netlink.h \
+ lib/if-notifier.c \
+ lib/if-notifier.h \
lib/netdev-linux.c \
lib/netdev-linux.h \
lib/netlink-notifier.c \
@@ -379,11 +382,13 @@ endif
if ESX
lib_libopenvswitch_la_SOURCES += \
-lib/route-table-stub.c
+ lib/route-table-stub.c \
+ lib/if-notifier-stub.c
endif
if HAVE_IF_DL
lib_libopenvswitch_la_SOURCES += \
+ lib/if-notifier-bsd.c \
lib/netdev-bsd.c \
lib/rtbsd.c \
lib/rtbsd.h \
diff --git a/lib/if-notifier-bsd.c b/lib/if-notifier-bsd.c
new file mode 100644
index 000..a1c3d0e
--- /dev/null
+++ b/lib/if-notifier-bsd.c
@@ -0,0 +1,69 @@
+/*
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include
+#include "if-notifier.h"
+#include "rtbsd.h"
+#include "util.h"
+
+struct if_notifier {
+struct rtbsd_notifier notifier;
+if_notify_func *cb;
+void *aux;
+};
+
+static void if_notifier_cb(const struct rtbsd_change *change OVS_UNUSED,
+ void *aux)
+{
+struct if_notifier *notifier;
+notifier = aux;
+notifier->cb(notifier->aux);
+}
+
+struct if_notifier *
+if_notifier_create(if_notify_func *cb, void *aux)
+{
+struct if_notifier *notifier;
+int ret;
+notifier = xzalloc(sizeof *notifier);
+notifier->cb = cb;
+notifier->aux = aux;
+ret = rtbsd_notifier_register(¬ifier->notifier, if_notifier_cb,
+ notifier);
+if (ret) {
+free(notifier);
+return NULL;
+}
+return notifier;
+}
+
+void if_notifier_destroy(struct if_notifier *notifier)
+{
+if (notifier) {
+rtbsd_notifier_unregister(¬ifier->notifier);
+free(notifier);
+}
+}
+
+void if_notifier_run(void)
+{
+rtbsd_notifier_run();
+}
+
+void if_notifier_wait(void)
+{
+rtbsd_notifier_wait();
+}
diff --git a/lib/if-notifier-stub.c b/lib/if-notifier-stub.c
new file mode 100644
index 000..ffd5876
--- /dev/null
+++ b/lib/if-notifier-stub.c
@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include
+#include "if-notifier.h"
+#include
+#include "compiler.h"
+
+struct if_notifier *
+if_notifier_create(if_notify_func *cb OVS_UNUSED, void *aux OVS_UNUSED)
+{
+return NULL;
+}
+
+void if_notifier_destroy(struct if_notifier *notifier OVS_UNUSED)
+{
+}
+
+void if_notifier_run(void)
+{
+}
+
+void if_notifier_wait(void)
+{
+}
diff --git a/lib/if-notifier.c b/lib/if-notifier.c
new file mode 100644
Re: [ovs-dev] Adding a new file to ovs-master
On Wed, Aug 05, 2015 at 03:55:11AM +, ravali.bu...@wipro.com wrote:
> Hi Russell & Thomas,
>
> When I am adding the new header file in bridge.c file depending on the order
> of the header file I am seeing different errors i.e. if I place the header
> file in the end of header file section i.e. before the declaration of VLOG
> module declaration then I am facing the issue like
>
> ./include/openvswitch/vlog.h:284:16: error: expected ';', identifier or '('
> before 'struct'
> extern struct vlog_module VLM_##MODULE;
>
> If I place the header file in the middle of the header file section ie before
> seq.h I am facing the error like
>
> lib/seq.h:121:1: error: expected ';', identifier or '(' before 'struct'
> struct seq *seq_create(void);
>
> Depending on the placement of the header file in declaration section I am
> facing different types of errors related to next header file.
>
> Can you please let me know how to proceed further.
>
> Thanks in advance.
>
> Regards,
> Ravali
Hi, Ravali.
That indicates the problem is likely in your new header file. Can you post your
changes to the list, or some git repository somewhere, or a pastebin?
As already mentioned to you, best thing would be to send it as a patch to the
mailing list, pointing out that it doesn't build.
Best of luck.
Cascardo.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 2/2] ovs-vsctl: add command to delete transient ports from bridge
When using virtualization, new ports are created and removed all the time. These
ports do not persist after a system reboot, for example. They may be created
again by the virtualization manager, but that will happen after the vswitch is
already running, and the virtualization manager will add them again to the
bridge.
If a reboot happens without properly deleting such ports, all kinds of errors
will happen. The absence of the ports will be logged as errors, and adding those
ports again to the database will fail.
This patch introduces the notion of transient ports. Ports may be added as
transient, as a boolean in other_config smap. When openvswitch is restarted by
using ovs-ctl, all transient ports will be removed. If the system administrator
wants to remove all transient ports from a bridge, a new ovs-vsctl command,
del-transient-ports may be used.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
tests/ovs-vsctl.at | 21 +
utilities/ovs-ctl.in | 3 +++
utilities/ovs-vsctl.8.in | 3 +++
utilities/ovs-vsctl.c| 30 ++
vswitchd/vswitch.xml | 8
5 files changed, 65 insertions(+)
diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index fef7b88..f877070 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -304,6 +304,27 @@ CHECK_IFACES([b], [b1])
OVS_VSCTL_CLEANUP
AT_CLEANUP
+AT_SETUP([add-br a, add-transient-port a a1 a2, del-transient-ports a])
+AT_KEYWORDS([ovs-vsctl del-transient-ports])
+OVS_VSCTL_SETUP
+AT_CHECK([RUN_OVS_VSCTL(
+ [add-br a],
+ [--if-exists del-br b],
+ [add-port a a1],
+ [add-port a a2],
+ [set port a1 other_config:transient=true],
+ [set port a2 other_config:transient=true])], [0], [], [],
[OVS_VSCTL_CLEANUP])
+CHECK_BRIDGES([a, a, 0])
+CHECK_PORTS([a], [a1], [a2])
+CHECK_IFACES([a], [a1], [a2])
+AT_CHECK([RUN_OVS_VSCTL(
+ [del-transient-ports a])], [0], [], [], [OVS_VSCTL_CLEANUP])
+CHECK_BRIDGES([a, a, 0])
+CHECK_PORTS([a])
+CHECK_IFACES([a])
+OVS_VSCTL_CLEANUP
+AT_CLEANUP
+
AT_SETUP([add-br a, add-bond a bond0 a1 a2 a3])
AT_KEYWORDS([ovs-vsctl])
OVS_VSCTL_SETUP
diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
index 9bbbe0d..b08987b 100755
--- a/utilities/ovs-ctl.in
+++ b/utilities/ovs-ctl.in
@@ -219,6 +219,9 @@ start_ovsdb () {
ovs_vsctl del-br $bridge
done
fi
+for bridge in `ovs_vsctl list-br`; do
+ovs_vsctl del-transient-ports $bridge
+done
fi
}
diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
index 265ffde..6229312 100644
--- a/utilities/ovs-vsctl.8.in
+++ b/utilities/ovs-vsctl.8.in
@@ -323,6 +323,9 @@ no effect.
Prints the name of the bridge that contains \fIport\fR on standard
output.
.
+.IP "\fBdel\-transient\-ports \fIbridge\fR"
+Deletes ports configured as transient from the specified bridge.
+.
.SS "Interface Commands"
.
These commands examine the interfaces attached to an Open vSwitch
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index e177060..4526c90 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -380,6 +380,7 @@ Port commands (a bond is considered to be a single port):\n\
add-bond BRIDGE PORT IFACE... add bonded port PORT in BRIDGE from IFACES\n\
del-port [BRIDGE] PORT delete PORT (which may be bonded) from BRIDGE\n\
port-to-br PORT print name of bridge that contains PORT\n\
+ del-transient-ports BRIDGE delete transient ports from BRIDGE\n\
\n\
Interface commands (a bond consists of multiple interfaces):\n\
list-ifaces BRIDGE print the names of all interfaces on BRIDGE\n\
@@ -1678,6 +1679,33 @@ cmd_del_port(struct ctl_context *ctx)
}
static void
+pre_transient_ports(struct ctl_context *ctx)
+{
+pre_get_info(ctx);
+ovsdb_idl_add_column(ctx->idl, &ovsrec_port_col_other_config);
+}
+
+static void
+cmd_del_transient_ports(struct ctl_context *ctx)
+{
+struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
+const char *target = ctx->argv[1];
+struct vsctl_bridge *bridge;
+struct vsctl_port *port, *next_port;
+
+vsctl_context_populate_cache(ctx);
+bridge = find_bridge(vsctl_ctx, target, true);
+
+LIST_FOR_EACH_SAFE (port, next_port, ports_node, &bridge->ports) {
+struct ovsrec_port *cfg = port->port_cfg;
+bool transient = smap_get_bool(&cfg->other_config, "transient", false);
+if (transient) {
+del_port(vsctl_ctx, port);
+}
+}
+}
+
+static void
cmd_port_to_br(struct ctl_context *ctx)
{
struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
@@ -2732,6 +2760,8 @@ static const struct ctl_command_syntax vsctl_commands[] =
{
{"del-port", 1, 2, "[BRIDGE] PORT|IFACE", pre_get_info, cmd_del_port, NULL,
"--if-exists,--with-iface", RW},
{"port-to-br", 1, 1, "PORT", pr
[ovs-dev] [PATCH 1/2] ovs-ctl: fix indentation when deleting bridges
Signed-off-by: Thadeu Lima de Souza Cascardo
---
utilities/ovs-ctl.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
index 918be2c..9bbbe0d 100755
--- a/utilities/ovs-ctl.in
+++ b/utilities/ovs-ctl.in
@@ -216,7 +216,7 @@ start_ovsdb () {
set_system_ids || return 1
if test X"$DELETE_BRIDGES" = Xyes; then
for bridge in `ovs_vsctl list-br`; do
-ovs_vsctl del-br $bridge
+ovs_vsctl del-br $bridge
done
fi
fi
--
2.4.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/2] ovs-vsctl: add command to delete transient ports from bridge
On Thu, Aug 27, 2015 at 11:28:21AM -0700, Ben Pfaff wrote: > On Thu, Aug 27, 2015 at 12:07:32PM -0300, Thadeu Lima de Souza Cascardo wrote: > > When using virtualization, new ports are created and removed all the time. > > These > > ports do not persist after a system reboot, for example. They may be created > > again by the virtualization manager, but that will happen after the vswitch > > is > > already running, and the virtualization manager will add them again to the > > bridge. > > > > If a reboot happens without properly deleting such ports, all kinds of > > errors > > will happen. The absence of the ports will be logged as errors, and adding > > those > > ports again to the database will fail. > > > > This patch introduces the notion of transient ports. Ports may be added as > > transient, as a boolean in other_config smap. When openvswitch is restarted > > by > > using ovs-ctl, all transient ports will be removed. If the system > > administrator > > wants to remove all transient ports from a bridge, a new ovs-vsctl command, > > del-transient-ports may be used. > > > > Signed-off-by: Thadeu Lima de Souza Cascardo > > I would think that reboot should be distinguished from restart within a > boot. Restart can happen for a number of reasons, e.g. to upgrade OVS > userspace, to upgrade the OVS kernel module, to troubleshoot suspected > bugs. In most of these cases one would not want to delete all of these > "transient" ports. > > Therefore I'd consider implementing this feature as an option like the > existing --delete-bridges option, something like > --delete-transient-ports. The system would supply the option only on > the first start following boot. > > Actually, that makes me wonder whether --delete-bridges is the right > solution. It makes a lot of sense to start fresh from an empty set of > bridges at boot time because it provides a "clean slate" at each boot on > which to reproducibly build up a set of bridges and ports. It worked > well on XenServer when we originally introduced it and I wonder whether > it's the right model everywhere. That's reasonable when all configurations can be persisted by other methods. I believe that are scenarios where the system is capable of persisting some configurations but not others. In those cases, it's preferable to have an option like --delete-transient-ports then the alternatives: deleting all bridges or have those undesired failures. I will send a V2. Gurucharan, I think that solves your concern about restoring flows. Is that OK for you? Thanks. Cascardo. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2] ovs-vsctl: add command to delete transient ports from bridge
When using virtualization, new ports are created and removed all the time. These
ports do not persist after a system reboot, for example. They may be created
again by the virtualization manager, but that will happen after the vswitch is
already running, and the virtualization manager will add them again to the
bridge.
If a reboot happens without properly deleting such ports, all kinds of errors
will happen. The absence of the ports will be logged as errors, and adding those
ports again to the database will fail.
This patch introduces the notion of transient ports. Ports may be added as
transient, as a boolean in other_config smap. When openvswitch is restarted by
using --delete-transient-ports ovs-ctl option, all transient ports will be
removed. If the system administrator wants to remove all transient ports from a
bridge, a new ovs-vsctl command, del-transient-ports may be used.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
tests/ovs-vsctl.at | 21 +
utilities/ovs-ctl.in | 6 ++
utilities/ovs-vsctl.8.in | 3 +++
utilities/ovs-vsctl.c| 30 ++
vswitchd/vswitch.xml | 8
5 files changed, 68 insertions(+)
diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index fef7b88..f877070 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -304,6 +304,27 @@ CHECK_IFACES([b], [b1])
OVS_VSCTL_CLEANUP
AT_CLEANUP
+AT_SETUP([add-br a, add-transient-port a a1 a2, del-transient-ports a])
+AT_KEYWORDS([ovs-vsctl del-transient-ports])
+OVS_VSCTL_SETUP
+AT_CHECK([RUN_OVS_VSCTL(
+ [add-br a],
+ [--if-exists del-br b],
+ [add-port a a1],
+ [add-port a a2],
+ [set port a1 other_config:transient=true],
+ [set port a2 other_config:transient=true])], [0], [], [],
[OVS_VSCTL_CLEANUP])
+CHECK_BRIDGES([a, a, 0])
+CHECK_PORTS([a], [a1], [a2])
+CHECK_IFACES([a], [a1], [a2])
+AT_CHECK([RUN_OVS_VSCTL(
+ [del-transient-ports a])], [0], [], [], [OVS_VSCTL_CLEANUP])
+CHECK_BRIDGES([a, a, 0])
+CHECK_PORTS([a])
+CHECK_IFACES([a])
+OVS_VSCTL_CLEANUP
+AT_CLEANUP
+
AT_SETUP([add-br a, add-bond a bond0 a1 a2 a3])
AT_KEYWORDS([ovs-vsctl])
OVS_VSCTL_SETUP
diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
index 9bbbe0d..5beb0a2 100755
--- a/utilities/ovs-ctl.in
+++ b/utilities/ovs-ctl.in
@@ -219,6 +219,11 @@ start_ovsdb () {
ovs_vsctl del-br $bridge
done
fi
+if test X"$DELETE_TRANSIENT_PORTS" = Xyes; then
+for bridge in `ovs_vsctl list-br`; do
+ovs_vsctl del-transient-ports $bridge
+done
+fi
fi
}
@@ -536,6 +541,7 @@ set_defaults () {
SYSTEM_ID=
DELETE_BRIDGES=no
+DELETE_TRANSIENT_PORTS=no
DAEMON_CWD=/
FORCE_COREFILES=yes
diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
index 265ffde..6229312 100644
--- a/utilities/ovs-vsctl.8.in
+++ b/utilities/ovs-vsctl.8.in
@@ -323,6 +323,9 @@ no effect.
Prints the name of the bridge that contains \fIport\fR on standard
output.
.
+.IP "\fBdel\-transient\-ports \fIbridge\fR"
+Deletes ports configured as transient from the specified bridge.
+.
.SS "Interface Commands"
.
These commands examine the interfaces attached to an Open vSwitch
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index e177060..4526c90 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -380,6 +380,7 @@ Port commands (a bond is considered to be a single port):\n\
add-bond BRIDGE PORT IFACE... add bonded port PORT in BRIDGE from IFACES\n\
del-port [BRIDGE] PORT delete PORT (which may be bonded) from BRIDGE\n\
port-to-br PORT print name of bridge that contains PORT\n\
+ del-transient-ports BRIDGE delete transient ports from BRIDGE\n\
\n\
Interface commands (a bond consists of multiple interfaces):\n\
list-ifaces BRIDGE print the names of all interfaces on BRIDGE\n\
@@ -1678,6 +1679,33 @@ cmd_del_port(struct ctl_context *ctx)
}
static void
+pre_transient_ports(struct ctl_context *ctx)
+{
+pre_get_info(ctx);
+ovsdb_idl_add_column(ctx->idl, &ovsrec_port_col_other_config);
+}
+
+static void
+cmd_del_transient_ports(struct ctl_context *ctx)
+{
+struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
+const char *target = ctx->argv[1];
+struct vsctl_bridge *bridge;
+struct vsctl_port *port, *next_port;
+
+vsctl_context_populate_cache(ctx);
+bridge = find_bridge(vsctl_ctx, target, true);
+
+LIST_FOR_EACH_SAFE (port, next_port, ports_node, &bridge->ports) {
+struct ovsrec_port *cfg = port->port_cfg;
+bool transient = smap_get_bool(&cfg->other_config, "transient", false);
+if (transient) {
+del_port(vsctl_ctx, port);
+}
+}
+}
+
+static void
cmd_port_to_br(struct ctl_context *ctx)
{
struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
@@ -2732
Re: [ovs-dev] [PATCH v2] ovs-vsctl: add command to delete transient ports from bridge
On Fri, Aug 28, 2015 at 04:13:05PM -0700, Ben Pfaff wrote: > On Fri, Aug 28, 2015 at 04:31:40PM -0300, Thadeu Lima de Souza Cascardo wrote: > > When using virtualization, new ports are created and removed all the time. > > These > > ports do not persist after a system reboot, for example. They may be created > > again by the virtualization manager, but that will happen after the vswitch > > is > > already running, and the virtualization manager will add them again to the > > bridge. > > > > If a reboot happens without properly deleting such ports, all kinds of > > errors > > will happen. The absence of the ports will be logged as errors, and adding > > those > > ports again to the database will fail. > > > > This patch introduces the notion of transient ports. Ports may be added as > > transient, as a boolean in other_config smap. When openvswitch is restarted > > by > > using --delete-transient-ports ovs-ctl option, all transient ports will be > > removed. If the system administrator wants to remove all transient ports > > from a > > bridge, a new ovs-vsctl command, del-transient-ports may be used. > > > > Signed-off-by: Thadeu Lima de Souza Cascardo > > Do you think that it's worth having a special command for this, versus > e.g. > > for port in `ovs-vsctl --bare -- --columns=name find port > other-config:transient=true`; do > ovs_vsctl -- del-port "$port" > done The patch has evolved from code inside the bridge (which would not work), and, though doing something like this has crossed my mind, I ended up reusing the code I've written in ovs-vsctl, and from there, it was greatly simplified. I will send a v3 wrapping this up on a shell function. I don't think there is any need for a special command as of now. We can go back to it, if such a reason appears. Thanks. Cascardo. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v3] ovs-ctl: add option to delete transient ports
When using virtualization, new ports are created and removed all the time. These
ports do not persist after a system reboot, for example. They may be created
again by the virtualization manager, but that will happen after the vswitch is
already running, and the virtualization manager will add them again to the
bridge.
If a reboot happens without properly deleting such ports, all kinds of errors
will happen. The absence of the ports will be logged as errors, and adding those
ports again to the database will fail.
Deleting all bridges may not be an option, if the system cannot persist other
information outside of OVSDB.
This patch introduces the notion of transient ports. Ports may be added as
transient, as a boolean in other_config smap. When openvswitch is started by
using --delete-transient-ports ovs-ctl option, all transient ports will be
removed.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
utilities/ovs-ctl.8 | 11 ++-
utilities/ovs-ctl.in | 10 ++
vswitchd/vswitch.xml | 8
3 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/utilities/ovs-ctl.8 b/utilities/ovs-ctl.8
index c08c7db..9728d0d 100644
--- a/utilities/ovs-ctl.8
+++ b/utilities/ovs-ctl.8
@@ -90,10 +90,14 @@ Initializes a few values inside the database.
If the \fB\-\-delete\-bridges\fR option was used, deletes all of the
bridges from the database.
.
+.IP 6.
+If the \fB\-\-delete\-transient\-ports\fR option was used, deletes all ports
+that have other_config:transient set to true.
+.
.PP
The \fBstart\fR command skips the following step if
\fBovs\-vswitchd\fR is already running:
-.IP 6.
+.IP 7.
Starts \fBovs\-vswitchd\fR.
.
.SS "Options"
@@ -148,6 +152,11 @@ on every boot. This option supports that, by deleting all
Open
vSwitch bridges after starting \fBovsdb\-server\fR but before starting
\fBovs\-vswitchd\fR.
.
+.IP "\fB\-\-delete\-transient\-ports\fR"
+Deletes all ports that have the other_config:transient value set to true. This
+is important on certain environments where some ports are going to be recreated
+after reboot, but other ports need to be persisted in the database.
+.
.PP
The following options are less important:
.
diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
index 9bbbe0d..bca8c56 100755
--- a/utilities/ovs-ctl.in
+++ b/utilities/ovs-ctl.in
@@ -177,6 +177,12 @@ check_force_cores () {
fi
}
+del_transient_ports () {
+for port in `ovs-vsctl --bare -- --columns=name find port
other_config:transient=true`; do
+ovs_vsctl -- del-port "$port"
+done
+}
+
start_ovsdb () {
check_force_cores
@@ -219,6 +225,9 @@ start_ovsdb () {
ovs_vsctl del-br $bridge
done
fi
+if test X"$DELETE_TRANSIENT_PORTS" = Xyes; then
+del_transient_ports
+fi
fi
}
@@ -536,6 +545,7 @@ set_defaults () {
SYSTEM_ID=
DELETE_BRIDGES=no
+DELETE_TRANSIENT_PORTS=no
DAEMON_CWD=/
FORCE_COREFILES=yes
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 457f34a..0ab4a9a 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -1659,6 +1659,14 @@
fake-bridge-,
e.g. fake-bridge-xs-network-uuids.
+
+
+
+ If set to true, the port will be removed when
+ ovs-ctl start --delete-transient-ports is used.
+
+
--
2.4.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 2/2] ovs-router: include ovs-router.h as first header
Signed-off-by: Thadeu Lima de Souza Cascardo --- lib/ovs-router.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/ovs-router.c b/lib/ovs-router.c index d4a5c35..7493c79 100644 --- a/lib/ovs-router.c +++ b/lib/ovs-router.c @@ -14,6 +14,8 @@ * limitations under the License. */ +#include "ovs-router.h" + #include #include #include @@ -34,7 +36,6 @@ #include "netdev.h" #include "packets.h" #include "seq.h" -#include "ovs-router.h" #include "ovs-thread.h" #include "route-table.h" #include "unixctl.h" -- 2.4.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/2] ovs-router: add missing ds_destroy after ds was put
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/ovs-router.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/ovs-router.c b/lib/ovs-router.c
index df55bb4..d4a5c35 100644
--- a/lib/ovs-router.c
+++ b/lib/ovs-router.c
@@ -278,6 +278,7 @@ ovs_router_lookup_cmd(struct unixctl_conn *conn, int argc
OVS_UNUSED,
ds_put_format(&ds, "gateway " IP_FMT "\n", IP_ARGS(gw));
ds_put_format(&ds, "dev %s\n", iface);
unixctl_command_reply(conn, ds_cstr(&ds));
+ds_destroy(&ds);
} else {
unixctl_command_reply(conn, "Not found");
}
--
2.4.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] doc: document feature deprecation and removal process
On Sat, Sep 19, 2015 at 01:22:39PM -0700, Ansis Atteka wrote: > It seems that we haven't defined clear process on how features should > be removed from OVS. This patch attempts to document this process. > > Singed-off-by: Ansis Atteka > --- > CONTRIBUTING.md | 31 +++ > 1 file changed, 31 insertions(+) > > diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md > index 12cb7dc..12d2a77 100644 > --- a/CONTRIBUTING.md > +++ b/CONTRIBUTING.md > @@ -24,6 +24,8 @@ In particular: > >- A patch that adds or removes user-visible features should > also update the appropriate user documentation or manpages. > +Check "Feature Deprecation Guidelines" section in this document > +if you intend to remove user-visible feature. > > Testing is also important: > > @@ -263,6 +265,35 @@ certifies the following: > maintained indefinitely and may be redistributed consistent with > this project or the open source license(s) involved. > > +Feature Deprecation Guidelines > +-- > + > +Open vSwitch is intended to be user friendly. This means that under > +normal circumstances we don't abruptly remove features from OVS that > +some users might still be using. Otherwise, if we would, then we would > +possibly break our user setup when they upgrade and would receive bug > +reports. > + > +Typical process to deprecate a feature in Open vSwitch is to: > + > +(a) Mention deprecation of a feature in the NEWS file. Also, mention > +expected release or absolute time when this feature would be removed > +from OVS altogether. Don't use relative time (e.g. "in 6 months") > +because that is not clearly interpretable. > + > +(b) If Open vSwitch is configured to use deprecated feature it should > print > +a warning message to the log files clearly indicating that feature is > +deprecated and that use of it should be avoided. > + > +(c) If this feature is mentioned in man pages, then add "Deprecated" > keyword > +to it. > + > +Also, if there is alternative feature to the one that is about to be marked > +as deprecated, then mention it in (a), (b) and (c) as well. > + > +Remember to followup and acctually remove the feature from OVS codebase > +once deprecation grace period has expired! > + > Comments > > > -- > 2.1.4 Should it make it clear that deprecation before removal should be part of a release? Otherwise, users will not notice the deprecation before the feature is removed. Cascardo. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] doc: document feature deprecation and removal process
On Mon, Sep 28, 2015 at 11:17:16AM -0700, Ansis Atteka wrote: > On Tue, Sep 22, 2015 at 4:21 AM, Thadeu Lima de Souza Cascardo > wrote: > > On Sat, Sep 19, 2015 at 01:22:39PM -0700, Ansis Atteka wrote: > >> It seems that we haven't defined clear process on how features should > >> be removed from OVS. This patch attempts to document this process. > >> > >> Singed-off-by: Ansis Atteka > >> --- > >> CONTRIBUTING.md | 31 +++ > >> 1 file changed, 31 insertions(+) > >> > >> diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md > >> index 12cb7dc..12d2a77 100644 > >> --- a/CONTRIBUTING.md > >> +++ b/CONTRIBUTING.md > >> @@ -24,6 +24,8 @@ In particular: > >> > >>- A patch that adds or removes user-visible features should > >> also update the appropriate user documentation or manpages. > >> +Check "Feature Deprecation Guidelines" section in this document > >> +if you intend to remove user-visible feature. > >> > >> Testing is also important: > >> > >> @@ -263,6 +265,35 @@ certifies the following: > >> maintained indefinitely and may be redistributed consistent with > >> this project or the open source license(s) involved. > >> > >> +Feature Deprecation Guidelines > >> +-- > >> + > >> +Open vSwitch is intended to be user friendly. This means that under > >> +normal circumstances we don't abruptly remove features from OVS that > >> +some users might still be using. Otherwise, if we would, then we would > >> +possibly break our user setup when they upgrade and would receive bug > >> +reports. > >> + > >> +Typical process to deprecate a feature in Open vSwitch is to: > >> + > >> +(a) Mention deprecation of a feature in the NEWS file. Also, mention > >> +expected release or absolute time when this feature would be > >> removed > >> +from OVS altogether. Don't use relative time (e.g. "in 6 months") > >> +because that is not clearly interpretable. > >> + > >> +(b) If Open vSwitch is configured to use deprecated feature it should > >> print > >> +a warning message to the log files clearly indicating that > >> feature is > >> +deprecated and that use of it should be avoided. > >> + > >> +(c) If this feature is mentioned in man pages, then add "Deprecated" > >> keyword > >> +to it. > >> + > >> +Also, if there is alternative feature to the one that is about to be > >> marked > >> +as deprecated, then mention it in (a), (b) and (c) as well. > >> + > >> +Remember to followup and acctually remove the feature from OVS codebase > >> +once deprecation grace period has expired! > >> + > >> Comments > >> > >> > >> -- > >> 2.1.4 > > > > Should it make it clear that deprecation before removal should be part of a > > release? Otherwise, users will not notice the deprecation before the > > feature is > > removed. > > Sorry for late response, I was off last week. If I understand > correctly, then you wanted this document to state more clearly that > "feature deprecation" and "feature removal" must be done in different > releases. I think wording "release after" should help to comprehend > that. So how about following wording: > > "Remember to followup and actually remove the feature from OVS codebase > in one of the next releases after deprecation grace period has expired." Well, that's about the feature removal. I mean the deprecation itself must be part of a release. For example, the feature should only be removed from 3.1, if the deprecation warning has been part of 3.0. Cascardo. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [RFC PATCH 2/7] Provide functions to work with IPv4-mapped IPv6 addresses.
Move in6_addr_set_mapped_ipv4 out of mcast-snooping code to packets.h and
provide an in6_addr_get_mapped_ipv4 function that gets the corresponding IPv4
address or the ANY address if it's not IPv4 mapped.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/mcast-snooping.c | 9 -
lib/packets.h| 21 +
2 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/lib/mcast-snooping.c b/lib/mcast-snooping.c
index ba4141e..ff2382d 100644
--- a/lib/mcast-snooping.c
+++ b/lib/mcast-snooping.c
@@ -125,15 +125,6 @@ mcast_snooping_lookup(const struct mcast_snooping *ms,
return NULL;
}
-static inline void
-in6_addr_set_mapped_ipv4(struct in6_addr *addr, ovs_be32 ip4)
-{
-union ovs_16aligned_in6_addr *taddr = (void *) addr;
-memset(taddr->be16, 0, sizeof(taddr->be16));
-taddr->be16[5] = OVS_BE16_MAX;
-put_16aligned_be32(&taddr->be32[3], ip4);
-}
-
struct mcast_group *
mcast_snooping_lookup4(const struct mcast_snooping *ms, ovs_be32 ip4,
uint16_t vlan)
diff --git a/lib/packets.h b/lib/packets.h
index 4fb1427..d55c718 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -28,6 +28,7 @@
#include "random.h"
#include "hash.h"
#include "tun-metadata.h"
+#include "unaligned.h"
#include "util.h"
struct dp_packet;
@@ -870,6 +871,26 @@ static inline bool ipv6_is_all_hosts(const struct in6_addr
*addr) {
return ipv6_addr_equals(addr, &in6addr_all_hosts);
}
+static inline void
+in6_addr_set_mapped_ipv4(struct in6_addr *addr, ovs_be32 ip4)
+{
+union ovs_16aligned_in6_addr *taddr = (void *) addr;
+memset(taddr->be16, 0, sizeof(taddr->be16));
+taddr->be16[5] = OVS_BE16_MAX;
+put_16aligned_be32(&taddr->be32[3], ip4);
+}
+
+static inline ovs_be32
+in6_addr_get_mapped_ipv4(const struct in6_addr *addr)
+{
+union ovs_16aligned_in6_addr *taddr = (void *) addr;
+if (IN6_IS_ADDR_V4MAPPED(addr)) {
+return get_16aligned_be32(&taddr->be32[3]);
+} else {
+return INADDR_ANY;
+}
+}
+
static inline bool dl_type_is_ip_any(ovs_be16 dl_type)
{
return dl_type == htons(ETH_TYPE_IP)
--
2.4.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [RFC PATCH 0/7] IPv6 userspace tunnel support code
This patchset introduces IPv6 support on some of the code used by userspace tunneling, including routing tables, ARP (now also Neighbor Discovery) spoofing and tunnel ports. Most of it uses the strategy of storing IPv4 addresses as IPv4-mapped IPv6 addresses. I am sending this as an RFC to check if that's okay to move forward with that. Please, send your comments. I included my Sign-off in case it's good enough for inclusion. So far, this patchset does not affect either IPv4 or IPv6 tunneling support, but it's necessary work for IPv6, which I am working on. I used some of the previous work by Jiri Benc, and I am working on some of the other code he posted last May. Thanks. Cascardo. Jiri Benc (2): lib: add ipv6 helper functions netlink: helper functions for ipv6 address in netlink attrs Thadeu Lima de Souza Cascardo (5): tnl-ports: Include tnl-ports.h as first header. Provide functions to work with IPv4-mapped IPv6 addresses. route: support IPv6 and use IPv4-mapped addresses tnl-arp-cache: add IPv6 Neighbor Discovery support tnl-ports: add IPv6 support lib/mcast-snooping.c | 9 --- lib/netdev-vport.c | 2 +- lib/netlink.c| 20 ++ lib/netlink.h| 6 ++ lib/ovs-router.c | 162 +-- lib/ovs-router.h | 8 ++- lib/packets.h| 29 lib/route-table.c| 104 +++ lib/tnl-arp-cache.c | 97 +- lib/tnl-arp-cache.h | 5 ++ lib/tnl-ports.c | 76 +++- ofproto/ofproto-dpif-sflow.c | 2 +- ofproto/ofproto-dpif-xlate.c | 3 +- tests/tunnel-push-pop.at | 8 +-- 14 files changed, 398 insertions(+), 133 deletions(-) -- 2.4.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [RFC PATCH 3/7] lib: add ipv6 helper functions
From: Jiri Benc
ipv6_addr_is_set is going to be used by next patches.
[cascardo: compare with in6addr_any in ipv6_addr_is_set]
[cascardo: keep only ipv6_addr_is_* functions]
Signed-off-by: Jiri Benc
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/packets.h | 8
1 file changed, 8 insertions(+)
diff --git a/lib/packets.h b/lib/packets.h
index d55c718..e841fb1 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -871,6 +871,14 @@ static inline bool ipv6_is_all_hosts(const struct in6_addr
*addr) {
return ipv6_addr_equals(addr, &in6addr_all_hosts);
}
+static inline bool ipv6_addr_is_set(const struct in6_addr *addr) {
+return !ipv6_addr_equals(addr, &in6addr_any);
+}
+
+static inline bool ipv6_addr_is_multicast(const struct in6_addr *ip) {
+return ip->s6_addr[0] == 0xff;
+}
+
static inline void
in6_addr_set_mapped_ipv4(struct in6_addr *addr, ovs_be32 ip4)
{
--
2.4.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [RFC PATCH 6/7] tnl-arp-cache: add IPv6 Neighbor Discovery support
Uses IPv4-mapped IPv6 addresses to store IPv4 addresses, and add support for
Neighbor Discovery snooping.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/tnl-arp-cache.c | 97
lib/tnl-arp-cache.h | 5 +++
ofproto/ofproto-dpif-xlate.c | 1 +
tests/tunnel-push-pop.at | 8 ++--
4 files changed, 89 insertions(+), 22 deletions(-)
diff --git a/lib/tnl-arp-cache.c b/lib/tnl-arp-cache.c
index ddfd26f..0ced4f6 100644
--- a/lib/tnl-arp-cache.c
+++ b/lib/tnl-arp-cache.c
@@ -14,8 +14,11 @@
* limitations under the License.
*/
+#include "tnl-arp-cache.h"
+
#include
#include
+#include
#include
#include "bitmap.h"
@@ -32,7 +35,6 @@
#include "seq.h"
#include "socket-util.h"
#include "timeval.h"
-#include "tnl-arp-cache.h"
#include "unaligned.h"
#include "unixctl.h"
#include "util.h"
@@ -44,7 +46,7 @@
struct tnl_arp_entry {
struct cmap_node cmap_node;
-ovs_be32 ip;
+struct in6_addr ip;
struct eth_addr mac;
time_t expires; /* Expiration time. */
char br_name[IFNAMSIZ];
@@ -53,13 +55,21 @@ struct tnl_arp_entry {
static struct cmap table;
static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER;
+static uint32_t
+tnl_arp_hash(const struct in6_addr *ip)
+{
+return hash_bytes(ip->s6_addr, 16, 0);
+}
+
static struct tnl_arp_entry *
-tnl_arp_lookup__(const char br_name[IFNAMSIZ], ovs_be32 dst)
+tnl_arp_lookup__(const char br_name[IFNAMSIZ], const struct in6_addr *dst)
{
struct tnl_arp_entry *arp;
+uint32_t hash;
-CMAP_FOR_EACH_WITH_HASH (arp, cmap_node, (OVS_FORCE uint32_t) dst, &table)
{
-if (arp->ip == dst && !strcmp(arp->br_name, br_name)) {
+hash = tnl_arp_hash(dst);
+CMAP_FOR_EACH_WITH_HASH (arp, cmap_node, hash, &table) {
+if (ipv6_addr_equals(&arp->ip, dst) && !strcmp(arp->br_name, br_name))
{
arp->expires = time_now() + ARP_ENTRY_DEFAULT_IDLE_TIME;
return arp;
}
@@ -73,8 +83,11 @@ tnl_arp_lookup(const char br_name[IFNAMSIZ], ovs_be32 dst,
{
struct tnl_arp_entry *arp;
int res = ENOENT;
+struct in6_addr dst6;
-arp = tnl_arp_lookup__(br_name, dst);
+in6_addr_set_mapped_ipv4(&dst6, dst);
+
+arp = tnl_arp_lookup__(br_name, &dst6);
if (arp) {
*mac = arp->mac;
res = 0;
@@ -83,6 +96,21 @@ tnl_arp_lookup(const char br_name[IFNAMSIZ], ovs_be32 dst,
return res;
}
+int
+tnl_nd_lookup(const char br_name[IFNAMSIZ], const struct in6_addr *dst,
+ struct eth_addr *mac)
+{
+struct tnl_arp_entry *arp;
+int res = ENOENT;
+
+arp = tnl_arp_lookup__(br_name, dst);
+if (arp) {
+*mac = arp->mac;
+res = 0;
+}
+return res;
+}
+
static void
arp_entry_free(struct tnl_arp_entry *arp)
{
@@ -92,12 +120,14 @@ arp_entry_free(struct tnl_arp_entry *arp)
static void
tnl_arp_delete(struct tnl_arp_entry *arp)
{
-cmap_remove(&table, &arp->cmap_node, (OVS_FORCE uint32_t) arp->ip);
+uint32_t hash = tnl_arp_hash(&arp->ip);
+cmap_remove(&table, &arp->cmap_node, hash);
ovsrcu_postpone(arp_entry_free, arp);
}
static void
-tnl_arp_set(const char name[IFNAMSIZ], ovs_be32 dst, const struct eth_addr mac)
+tnl_arp_set__(const char name[IFNAMSIZ], const struct in6_addr *dst,
+ const struct eth_addr mac)
{
ovs_mutex_lock(&mutex);
struct tnl_arp_entry *arp = tnl_arp_lookup__(name, dst);
@@ -113,14 +143,24 @@ tnl_arp_set(const char name[IFNAMSIZ], ovs_be32 dst,
const struct eth_addr mac)
arp = xmalloc(sizeof *arp);
-arp->ip = dst;
+arp->ip = *dst;
arp->mac = mac;
arp->expires = time_now() + ARP_ENTRY_DEFAULT_IDLE_TIME;
ovs_strlcpy(arp->br_name, name, sizeof arp->br_name);
-cmap_insert(&table, &arp->cmap_node, (OVS_FORCE uint32_t) arp->ip);
+uint32_t hash = tnl_arp_hash(&arp->ip);
+cmap_insert(&table, &arp->cmap_node, hash);
ovs_mutex_unlock(&mutex);
}
+static void
+tnl_arp_set(const char name[IFNAMSIZ], ovs_be32 dst,
+const struct eth_addr mac)
+{
+struct in6_addr dst6;
+in6_addr_set_mapped_ipv4(&dst6, dst);
+tnl_arp_set__(name, &dst6, mac);
+}
+
int
tnl_arp_snoop(const struct flow *flow, struct flow_wildcards *wc,
const char name[IFNAMSIZ])
@@ -139,6 +179,26 @@ tnl_arp_snoop(const struct flow *flow, struct
flow_wildcards *wc,
return 0;
}
+int
+tnl_nd_snoop(const struct flow *flow, struct flow_wildcards *wc,
+ const char name[IFNAMSIZ])
+{
+if (flow->dl_type != htons(ETH_TYPE_IPV6) ||
+flow->nw_proto != IPPROTO_ICMPV6 ||
+flow->tp_dst != htons(0) ||
+flow->tp_src != htons(ND_NEIGHBOR_AD
[ovs-dev] [RFC PATCH 7/7] tnl-ports: add IPv6 support
Retrieve interfaces IPv6 addresses, and store IPv4 addresses as IPv4-mapped IPv6
addresses.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/tnl-ports.c | 72 +
1 file changed, 57 insertions(+), 15 deletions(-)
diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c
index 60dc06f..c82f018 100644
--- a/lib/tnl-ports.c
+++ b/lib/tnl-ports.c
@@ -39,7 +39,8 @@ static struct classifier cls; /* Tunnel ports. */
struct ip_device {
struct netdev *dev;
struct eth_addr mac;
-ovs_be32 addr;
+ovs_be32 addr4;
+struct in6_addr addr6;
uint64_t change_seq;
struct ovs_list node;
char dev_name[IFNAMSIZ];
@@ -80,13 +81,18 @@ tnl_port_free(struct tnl_port_in *p)
static void
tnl_port_init_flow(struct flow *flow, struct eth_addr mac,
- ovs_be32 addr, ovs_be16 udp_port)
+ struct in6_addr *addr, ovs_be16 udp_port)
{
memset(flow, 0, sizeof *flow);
-flow->dl_type = htons(ETH_TYPE_IP);
flow->dl_dst = mac;
-flow->nw_dst = addr;
+if (IN6_IS_ADDR_V4MAPPED(addr)) {
+flow->dl_type = htons(ETH_TYPE_IP);
+flow->nw_dst = in6_addr_get_mapped_ipv4(addr);
+} else {
+flow->dl_type = htons(ETH_TYPE_IPV6);
+flow->ipv6_dst = *addr;
+}
if (udp_port) {
flow->nw_proto = IPPROTO_UDP;
@@ -97,7 +103,7 @@ tnl_port_init_flow(struct flow *flow, struct eth_addr mac,
}
static void
-map_insert(odp_port_t port, struct eth_addr mac, ovs_be32 addr,
+map_insert(odp_port_t port, struct eth_addr mac, struct in6_addr *addr,
ovs_be16 udp_port, const char dev_name[])
{
const struct cls_rule *cr;
@@ -121,7 +127,11 @@ map_insert(odp_port_t port, struct eth_addr mac, ovs_be32
addr,
match.wc.masks.nw_proto = 0xff;
match.wc.masks.nw_frag = 0xff; /* XXX: No fragments support. */
match.wc.masks.tp_dst = OVS_BE16_MAX;
-match.wc.masks.nw_dst = OVS_BE32_MAX;
+if (IN6_IS_ADDR_V4MAPPED(addr)) {
+match.wc.masks.nw_dst = OVS_BE32_MAX;
+} else {
+match.wc.masks.ipv6_dst = in6addr_exact;
+}
match.wc.masks.vlan_tci = OVS_BE16_MAX;
memset(&match.wc.masks.dl_dst, 0xff, sizeof (struct eth_addr));
@@ -154,8 +164,16 @@ tnl_port_map_insert(odp_port_t port,
list_insert(&port_list, &p->node);
LIST_FOR_EACH(ip_dev, node, &addr_list) {
-map_insert(p->port, ip_dev->mac, ip_dev->addr,
- p->udp_port, p->dev_name);
+if (ip_dev->addr4 != INADDR_ANY) {
+struct in6_addr addr4;
+in6_addr_set_mapped_ipv4(&addr4, ip_dev->addr4);
+map_insert(p->port, ip_dev->mac, &addr4,
+ p->udp_port, p->dev_name);
+}
+if (ipv6_addr_is_set(&ip_dev->addr6)) {
+map_insert(p->port, ip_dev->mac, &ip_dev->addr6,
+ p->udp_port, p->dev_name);
+}
}
out:
@@ -175,7 +193,7 @@ tnl_port_unref(const struct cls_rule *cr)
}
static void
-map_delete(struct eth_addr mac, ovs_be32 addr, ovs_be16 udp_port)
+map_delete(struct eth_addr mac, struct in6_addr *addr, ovs_be16 udp_port)
{
const struct cls_rule *cr;
struct flow flow;
@@ -206,7 +224,14 @@ tnl_port_map_delete(ovs_be16 udp_port)
goto out;
}
LIST_FOR_EACH(ip_dev, node, &addr_list) {
-map_delete(ip_dev->mac, ip_dev->addr, udp_port);
+if (ip_dev->addr4 != INADDR_ANY) {
+struct in6_addr addr4;
+in6_addr_set_mapped_ipv4(&addr4, ip_dev->addr4);
+map_delete(ip_dev->mac, &addr4, udp_port);
+}
+if (ipv6_addr_is_set(&ip_dev->addr6)) {
+map_delete(ip_dev->mac, &ip_dev->addr6, udp_port);
+}
}
free(p);
@@ -301,8 +326,16 @@ map_insert_ipdev(struct ip_device *ip_dev)
struct tnl_port *p;
LIST_FOR_EACH(p, node, &port_list) {
-map_insert(p->port, ip_dev->mac, ip_dev->addr,
- p->udp_port, p->dev_name);
+if (ip_dev->addr4 != INADDR_ANY) {
+struct in6_addr addr4;
+in6_addr_set_mapped_ipv4(&addr4, ip_dev->addr4);
+map_insert(p->port, ip_dev->mac, &addr4,
+ p->udp_port, p->dev_name);
+}
+if (ipv6_addr_is_set(&ip_dev->addr6)) {
+map_insert(p->port, ip_dev->mac, &ip_dev->addr6,
+ p->udp_port, p->dev_name);
+}
}
}
@@ -313,6 +346,7 @@ insert_ipdev(const char dev_name[])
enum netdev_flags flags;
struct netdev *dev;
int error;
+int error4, error6;
error = netdev_open(dev_name, NULL, &dev);
if (error) {
@@ -332,8 +366,9 @@ insert_ipdev(const char dev_n
[ovs-dev] [RFC PATCH 5/7] route: support IPv6 and use IPv4-mapped addresses
This adds support for IPv6 in ovs-router and route-table. IPv4 is stored in
ovs-router using IPv4-mapped addresses.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/netdev-vport.c | 2 +-
lib/ovs-router.c | 162 +--
lib/ovs-router.h | 8 ++-
lib/route-table.c| 104 +++
ofproto/ofproto-dpif-sflow.c | 2 +-
ofproto/ofproto-dpif-xlate.c | 2 +-
6 files changed, 194 insertions(+), 86 deletions(-)
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index ff50563..07b72b3 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -321,7 +321,7 @@ tunnel_check_status_change__(struct netdev_vport *netdev)
iface[0] = '\0';
route = netdev->tnl_cfg.ip_dst;
-if (ovs_router_lookup(route, iface, &gw)) {
+if (ovs_router_lookup4(route, iface, &gw)) {
struct netdev *egress_netdev;
if (!netdev_open(iface, "system", &egress_netdev)) {
diff --git a/lib/ovs-router.c b/lib/ovs-router.c
index d6c7652..2f093e8 100644
--- a/lib/ovs-router.c
+++ b/lib/ovs-router.c
@@ -49,8 +49,8 @@ static struct classifier cls;
struct ovs_router_entry {
struct cls_rule cr;
char output_bridge[IFNAMSIZ];
-ovs_be32 gw;
-ovs_be32 nw_addr;
+struct in6_addr gw;
+struct in6_addr nw_addr;
uint8_t plen;
uint8_t priority;
};
@@ -66,10 +66,11 @@ ovs_router_entry_cast(const struct cls_rule *cr)
}
bool
-ovs_router_lookup(ovs_be32 ip_dst, char output_bridge[], ovs_be32 *gw)
+ovs_router_lookup(const struct in6_addr *ip6_dst, char output_bridge[],
+ struct in6_addr *gw)
{
const struct cls_rule *cr;
-struct flow flow = {.nw_dst = ip_dst};
+struct flow flow = {.ipv6_dst = *ip6_dst};
cr = classifier_lookup(&cls, CLS_MAX_VERSION, &flow, NULL);
if (cr) {
@@ -79,6 +80,20 @@ ovs_router_lookup(ovs_be32 ip_dst, char output_bridge[],
ovs_be32 *gw)
*gw = p->gw;
return true;
}
+return false;
+}
+
+bool
+ovs_router_lookup4(ovs_be32 ip_dst, char output_bridge[], ovs_be32 *gw)
+{
+struct in6_addr ip6_dst;
+struct in6_addr gw6;
+
+in6_addr_set_mapped_ipv4(&ip6_dst, ip_dst);
+if (ovs_router_lookup(&ip6_dst, output_bridge, &gw6)) {
+*gw = in6_addr_get_mapped_ipv4(&gw6);
+return true;
+}
return route_table_fallback_lookup(ip_dst, output_bridge, gw);
}
@@ -89,33 +104,37 @@ rt_entry_free(struct ovs_router_entry *p)
free(p);
}
-static void rt_init_match(struct match *match, ovs_be32 ip_dst, uint8_t plen)
+static void rt_init_match(struct match *match, const struct in6_addr *ip6_dst,
+ uint8_t plen)
{
-ovs_be32 mask;
+struct in6_addr dst;
+struct in6_addr mask;
-mask = be32_prefix_mask(plen);
+mask = ipv6_create_mask(plen);
-ip_dst &= mask; /* Clear out insignificant bits. */
+dst = ipv6_addr_bitand(ip6_dst, &mask);
memset(match, 0, sizeof *match);
-match->flow.nw_dst = ip_dst;
-match->wc.masks.nw_dst = mask;
+match->flow.ipv6_dst = dst;
+match->wc.masks.ipv6_dst = mask;
}
static void
-ovs_router_insert__(uint8_t priority, ovs_be32 ip_dst, uint8_t plen,
-const char output_bridge[],
-ovs_be32 gw)
+ovs_router_insert__(uint8_t priority, const struct in6_addr *ip6_dst,
+uint8_t plen, const char output_bridge[],
+const struct in6_addr *gw)
{
const struct cls_rule *cr;
struct ovs_router_entry *p;
struct match match;
-rt_init_match(&match, ip_dst, plen);
+rt_init_match(&match, ip6_dst, plen);
p = xzalloc(sizeof *p);
ovs_strlcpy(p->output_bridge, output_bridge, sizeof p->output_bridge);
-p->gw = gw;
-p->nw_addr = match.flow.nw_dst;
+if (ipv6_addr_is_set(gw)) {
+p->gw = *gw;
+}
+p->nw_addr = match.flow.ipv6_dst;
p->plen = plen;
p->priority = priority;
/* Longest prefix matches first. */
@@ -134,8 +153,8 @@ ovs_router_insert__(uint8_t priority, ovs_be32 ip_dst,
uint8_t plen,
}
void
-ovs_router_insert(ovs_be32 ip_dst, uint8_t plen, const char output_bridge[],
- ovs_be32 gw)
+ovs_router_insert(const struct in6_addr *ip_dst, uint8_t plen,
+ const char output_bridge[], const struct in6_addr *gw)
{
ovs_router_insert__(plen, ip_dst, plen, output_bridge, gw);
}
@@ -157,14 +176,14 @@ __rt_entry_delete(const struct cls_rule *cr)
}
static bool
-rt_entry_delete(uint8_t priority, ovs_be32 ip_dst, uint8_t plen)
+rt_entry_delete(uint8_t priority, const struct in6_addr *ip6_dst, uint8_t plen)
{
const struct cls_rule *cr;
struct cls_rule rule;
struct match match;
bool res = false;
-rt_init_match(&match, ip_dst, plen);
+rt_init_match(&mat
[ovs-dev] [RFC PATCH 4/7] netlink: helper functions for ipv6 address in netlink attrs
From: Jiri Benc
[cascardo: add NL_A_IPV6, used in next patch]
Signed-off-by: Jiri Benc
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/netlink.c | 20
lib/netlink.h | 6 ++
2 files changed, 26 insertions(+)
diff --git a/lib/netlink.c b/lib/netlink.c
index 09723b2..6bb3537 100644
--- a/lib/netlink.c
+++ b/lib/netlink.c
@@ -308,6 +308,15 @@ nl_msg_put_be64(struct ofpbuf *msg, uint16_t type,
ovs_be64 value)
nl_msg_put_unspec(msg, type, &value, sizeof value);
}
+/* Appends a Netlink attribute of the given 'type' and the given IPv6
+ * address order 'value' to 'msg'. */
+void
+nl_msg_put_in6_addr(struct ofpbuf *msg, uint16_t type,
+const struct in6_addr *value)
+{
+nl_msg_put_unspec(msg, type, value, sizeof *value);
+}
+
/* Appends a Netlink attribute of the given 'type' and the given odp_port_t
* 'value' to 'msg'. */
void
@@ -603,6 +612,15 @@ nl_attr_get_be64(const struct nlattr *nla)
return get_32aligned_be64(x);
}
+/* Returns the IPv6 address value in 'nla''s payload.
+ *
+ * Asserts that 'nla''s payload is at least 16 bytes long. */
+struct in6_addr
+nl_attr_get_in6_addr(const struct nlattr *nla)
+{
+return NL_ATTR_GET_AS(nla, struct in6_addr);
+}
+
/* Returns the 32-bit odp_port_t value in 'nla''s payload.
*
* Asserts that 'nla''s payload is at least 4 bytes long. */
@@ -643,6 +661,7 @@ min_attr_len(enum nl_attr_type type)
case NL_A_U64: return 8;
case NL_A_STRING: return 1;
case NL_A_FLAG: return 0;
+case NL_A_IPV6: return 16;
case NL_A_NESTED: return 0;
case N_NL_ATTR_TYPES: default: OVS_NOT_REACHED();
}
@@ -661,6 +680,7 @@ max_attr_len(enum nl_attr_type type)
case NL_A_U64: return 8;
case NL_A_STRING: return SIZE_MAX;
case NL_A_FLAG: return SIZE_MAX;
+case NL_A_IPV6: return 16;
case NL_A_NESTED: return SIZE_MAX;
case N_NL_ATTR_TYPES: default: OVS_NOT_REACHED();
}
diff --git a/lib/netlink.h b/lib/netlink.h
index 6068f5d..eed71dd 100644
--- a/lib/netlink.h
+++ b/lib/netlink.h
@@ -31,6 +31,7 @@
* Linux-specific definitions for Netlink sockets, see netlink-socket.h.
*/
+#include
#include
#include
#include
@@ -69,6 +70,8 @@ void nl_msg_put_u64(struct ofpbuf *, uint16_t type, uint64_t
value);
void nl_msg_put_be16(struct ofpbuf *, uint16_t type, ovs_be16 value);
void nl_msg_put_be32(struct ofpbuf *, uint16_t type, ovs_be32 value);
void nl_msg_put_be64(struct ofpbuf *, uint16_t type, ovs_be64 value);
+void nl_msg_put_in6_addr(struct ofpbuf *msg, uint16_t type,
+ const struct in6_addr *value);
void nl_msg_put_odp_port(struct ofpbuf *, uint16_t type, odp_port_t value);
void nl_msg_put_string(struct ofpbuf *, uint16_t type, const char *value);
@@ -111,6 +114,7 @@ struct nlmsghdr *nl_msg_next(struct ofpbuf *buffer, struct
ofpbuf *msg);
#define NL_A_BE32_SIZE NL_ATTR_SIZE(sizeof(ovs_be32))
#define NL_A_BE64_SIZE NL_ATTR_SIZE(sizeof(ovs_be64))
#define NL_A_FLAG_SIZE NL_ATTR_SIZE(0)
+#define NL_A_IPV6_SIZE NL_ATTR_SIZE(sizeof(struct in6_addr))
bool nl_attr_oversized(size_t payload_size);
@@ -128,6 +132,7 @@ enum nl_attr_type
NL_A_BE64 = NL_A_U64,
NL_A_STRING,
NL_A_FLAG,
+NL_A_IPV6,
NL_A_NESTED,
N_NL_ATTR_TYPES
};
@@ -189,6 +194,7 @@ uint64_t nl_attr_get_u64(const struct nlattr *);
ovs_be16 nl_attr_get_be16(const struct nlattr *);
ovs_be32 nl_attr_get_be32(const struct nlattr *);
ovs_be64 nl_attr_get_be64(const struct nlattr *);
+struct in6_addr nl_attr_get_in6_addr(const struct nlattr *nla);
odp_port_t nl_attr_get_odp_port(const struct nlattr *);
const char *nl_attr_get_string(const struct nlattr *);
void nl_attr_get_nested(const struct nlattr *, struct ofpbuf *);
--
2.4.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [RFC PATCH 1/7] tnl-ports: Include tnl-ports.h as first header.
Signed-off-by: Thadeu Lima de Souza Cascardo --- lib/tnl-ports.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c index 59d5354..60dc06f 100644 --- a/lib/tnl-ports.c +++ b/lib/tnl-ports.c @@ -15,6 +15,9 @@ */ #include + +#include "tnl-ports.h" + #include #include @@ -26,7 +29,6 @@ #include "ovs-thread.h" #include "odp-util.h" #include "tnl-arp-cache.h" -#include "tnl-ports.h" #include "ovs-thread.h" #include "unixctl.h" #include "util.h" -- 2.4.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 3/7] tnl-neigh-cashe: tighten arp and nd snooping.
Fix subject: s/cashe/cache/.
On Tue, Mar 15, 2016 at 09:57:29AM -0700, Pravin B Shelar wrote:
> Currently arp and nd snooping is pretty loose. That causes
> unnecessary entries in neighbour cache. Following patch
> adds required checks.
>
> Signed-off-by: Pravin B Shelar
> ---
> lib/tnl-neigh-cache.c| 7 +--
> tests/ofproto-dpif.at| 2 +-
> tests/tunnel-push-pop.at | 4 ++--
> 3 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/lib/tnl-neigh-cache.c b/lib/tnl-neigh-cache.c
> index 0339b52..d95855a 100644
> --- a/lib/tnl-neigh-cache.c
> +++ b/lib/tnl-neigh-cache.c
> @@ -147,7 +147,9 @@ static int
> tnl_arp_snoop(const struct flow *flow, struct flow_wildcards *wc,
>const char name[IFNAMSIZ])
> {
> -if (flow->dl_type != htons(ETH_TYPE_ARP)) {
> +if (flow->dl_type != htons(ETH_TYPE_ARP) ||
> +flow->nw_proto != ARP_OP_REPLY ||
> +!memcmp(&flow->arp_sha, ð_addr_zero, sizeof (flow->arp_sha))) {
> return EINVAL;
There is eth_addr_is_zero.
> }
>
> @@ -167,7 +169,8 @@ tnl_nd_snoop(const struct flow *flow, struct
> flow_wildcards *wc,
> if (flow->dl_type != htons(ETH_TYPE_IPV6) ||
> flow->nw_proto != IPPROTO_ICMPV6 ||
> flow->tp_dst != htons(0) ||
> -flow->tp_src != htons(ND_NEIGHBOR_ADVERT)) {
> +flow->tp_src != htons(ND_NEIGHBOR_ADVERT) ||
> +!memcmp(&flow->arp_tha, ð_addr_zero, sizeof (flow->arp_tha))) {
Same here. In this particular case, I think it's important to note the cases
where an empty TLLs may occur, and why they don't matter for us.
In particular, here is what I wrote in my commit:
RFC4861 says Neighbor Advertisements sent in response to unicast Neighbor
Solicitations SHOULD include the Target link-layer address. However, Linux
doesn't. Responses to multicast NS MUST include it, which is what OVS sends.
So, the response to Solicitations sent by OVS will include the TLL address and
other Advertisements not including it can be ignored.
And I think that justifies for a separate check with a comment of its own,
mentioning this particular situation.
Thanks.
Cascardo.
> return EINVAL;
> }
>
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index 2d81711..72df90e 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -5483,7 +5483,7 @@ AT_CHECK([ovs-appctl ovs/route/add 1.1.2.92/24 br0],
> [0], [OK
> AT_CHECK([ovs-ofctl add-flow br0 action=normal])
>
> dnl Prime ARP Cache for 1.1.2.92
> -AT_CHECK([ovs-appctl netdev-dummy/receive br0
> 'recirc_id(0),in_port(100),eth(src=f8:bc:12:44:34:b6,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1.2.88,op=1,sha=f8:bc:12:44:34:b6,tha=00:00:00:00:00:00)'])
> +AT_CHECK([ovs-appctl netdev-dummy/receive br0
> 'recirc_id(0),in_port(100),eth(src=f8:bc:12:44:34:b6,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1.2.88,op=2,sha=f8:bc:12:44:34:b6,tha=00:00:00:00:00:00)'])
>
> dnl configure sflow on int-br only
> ovs-vsctl \
> diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at
> index 583abb5..a6118a8 100644
> --- a/tests/tunnel-push-pop.at
> +++ b/tests/tunnel-push-pop.at
> @@ -37,8 +37,8 @@ AT_CHECK([ovs-appctl ovs/route/add 1.1.2.92/24 br0], [0],
> [OK
> AT_CHECK([ovs-ofctl add-flow br0 action=normal])
>
> dnl Check ARP Snoop
> -AT_CHECK([ovs-appctl netdev-dummy/receive br0
> 'recirc_id(0),in_port(100),eth(src=f8:bc:12:44:34:b6,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1.2.88,op=1,sha=f8:bc:12:44:34:b6,tha=00:00:00:00:00:00)'])
> -AT_CHECK([ovs-appctl netdev-dummy/receive br0
> 'recirc_id(0),in_port(100),eth(src=f8:bc:12:44:34:b7,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.2.93,tip=1.1.2.88,op=1,sha=f8:bc:12:44:34:b7,tha=00:00:00:00:00:00)'])
> +AT_CHECK([ovs-appctl netdev-dummy/receive br0
> 'recirc_id(0),in_port(100),eth(src=f8:bc:12:44:34:b6,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1.2.88,op=2,sha=f8:bc:12:44:34:b6,tha=00:00:00:00:00:00)'])
> +AT_CHECK([ovs-appctl netdev-dummy/receive br0
> 'recirc_id(0),in_port(100),eth(src=f8:bc:12:44:34:b7,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.2.93,tip=1.1.2.88,op=2,sha=f8:bc:12:44:34:b7,tha=00:00:00:00:00:00)'])
>
> AT_CHECK([ovs-appctl tnl/neigh/show], [0], [dnl
> IPMAC Bridge
> --
> 2.5.0
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3 3/7] tnl-neigh-cache: tighten arp and nd snooping.
On Wed, Mar 23, 2016 at 02:56:55PM -0700, Pravin B Shelar wrote:
> Currently arp and nd snooping is pretty loose. That causes
> unnecessary entries in neighbour cache. Following patch
> adds required checks.
> Thanks Cascardo for detailed comment msg.
>
> CC: Thadeu Lima de Souza Cascardo
> Signed-off-by: Pravin B Shelar
Thanks for the change, Pravin.
Acked-by: Thadeu Lima de Souza Cascardo
> ---
> lib/tnl-neigh-cache.c| 13 -
> tests/ofproto-dpif.at| 2 +-
> tests/tunnel-push-pop.at | 4 ++--
> 3 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/lib/tnl-neigh-cache.c b/lib/tnl-neigh-cache.c
> index a62402c..8650e73 100644
> --- a/lib/tnl-neigh-cache.c
> +++ b/lib/tnl-neigh-cache.c
> @@ -147,7 +147,9 @@ static int
> tnl_arp_snoop(const struct flow *flow, struct flow_wildcards *wc,
>const char name[IFNAMSIZ])
> {
> -if (flow->dl_type != htons(ETH_TYPE_ARP)) {
> +if (flow->dl_type != htons(ETH_TYPE_ARP) ||
> +flow->nw_proto != ARP_OP_REPLY ||
> +eth_addr_is_zero(flow->arp_sha)) {
> return EINVAL;
> }
>
> @@ -170,6 +172,15 @@ tnl_nd_snoop(const struct flow *flow, struct
> flow_wildcards *wc,
> flow->tp_src != htons(ND_NEIGHBOR_ADVERT)) {
> return EINVAL;
> }
> +/* - RFC4861 says Neighbor Advertisements sent in response to unicast
> Neighbor
> + * Solicitations SHOULD include the Target link-layer address.
> However, Linux
> + * doesn't. So, the response to Solicitations sent by OVS will include
> the
> + * TLL address and other Advertisements not including it can be
> ignored.
> + * - OVS flow extract can set this field to zero in case of packet
> parsing errors.
> + * For details refer miniflow_extract()*/
> +if (eth_addr_is_zero(flow->arp_tha)) {
> +return EINVAL;
> +}
>
> memset(&wc->masks.ipv6_src, 0xff, sizeof wc->masks.ipv6_src);
> memset(&wc->masks.ipv6_dst, 0xff, sizeof wc->masks.ipv6_dst);
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index 0991444..da29ac2 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -5483,7 +5483,7 @@ AT_CHECK([ovs-appctl ovs/route/add 1.1.2.92/24 br0],
> [0], [OK
> AT_CHECK([ovs-ofctl add-flow br0 action=normal])
>
> dnl Prime ARP Cache for 1.1.2.92
> -AT_CHECK([ovs-appctl netdev-dummy/receive br0
> 'recirc_id(0),in_port(100),eth(src=f8:bc:12:44:34:b6,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1.2.88,op=1,sha=f8:bc:12:44:34:b6,tha=00:00:00:00:00:00)'])
> +AT_CHECK([ovs-appctl netdev-dummy/receive br0
> 'recirc_id(0),in_port(100),eth(src=f8:bc:12:44:34:b6,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1.2.88,op=2,sha=f8:bc:12:44:34:b6,tha=00:00:00:00:00:00)'])
>
> dnl configure sflow on int-br only
> ovs-vsctl \
> diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at
> index 583abb5..a6118a8 100644
> --- a/tests/tunnel-push-pop.at
> +++ b/tests/tunnel-push-pop.at
> @@ -37,8 +37,8 @@ AT_CHECK([ovs-appctl ovs/route/add 1.1.2.92/24 br0], [0],
> [OK
> AT_CHECK([ovs-ofctl add-flow br0 action=normal])
>
> dnl Check ARP Snoop
> -AT_CHECK([ovs-appctl netdev-dummy/receive br0
> 'recirc_id(0),in_port(100),eth(src=f8:bc:12:44:34:b6,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1.2.88,op=1,sha=f8:bc:12:44:34:b6,tha=00:00:00:00:00:00)'])
> -AT_CHECK([ovs-appctl netdev-dummy/receive br0
> 'recirc_id(0),in_port(100),eth(src=f8:bc:12:44:34:b7,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.2.93,tip=1.1.2.88,op=1,sha=f8:bc:12:44:34:b7,tha=00:00:00:00:00:00)'])
> +AT_CHECK([ovs-appctl netdev-dummy/receive br0
> 'recirc_id(0),in_port(100),eth(src=f8:bc:12:44:34:b6,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1.2.88,op=2,sha=f8:bc:12:44:34:b6,tha=00:00:00:00:00:00)'])
> +AT_CHECK([ovs-appctl netdev-dummy/receive br0
> 'recirc_id(0),in_port(100),eth(src=f8:bc:12:44:34:b7,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.2.93,tip=1.1.2.88,op=2,sha=f8:bc:12:44:34:b7,tha=00:00:00:00:00:00)'])
>
> AT_CHECK([ovs-appctl tnl/neigh/show], [0], [dnl
> IPMAC Bridge
> --
> 1.8.3.1
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] netdev: verify ifa_addr is not NULL when iterating over getifaddrs
Some point-to-point devices like TUN devices will not have an address, and while
iterating over ifaddrs, its ifa_addr will be NULL. This patch fixes a crash when
starting ovs-vswitchd on a system with such a device.
Signed-off-by: Thadeu Lima de Souza Cascardo
Fixes: a8704b502785 ("tunneling: Handle multiple ip address for given device.")
Cc: Pravin B Shelar
---
lib/netdev.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/lib/netdev.c b/lib/netdev.c
index 95fdbc7..928f46f 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -1882,12 +1882,14 @@ netdev_get_addrs(const char dev[], struct in6_addr
**paddr,
}
for (ifa = if_addr_list; ifa; ifa = ifa->ifa_next) {
-int family;
+if (ifa->ifa_addr != NULL) {
+int family;
-family = ifa->ifa_addr->sa_family;
-if (family == AF_INET || family == AF_INET6) {
-if (!strncmp(ifa->ifa_name, dev, IFNAMSIZ)) {
-cnt++;
+family = ifa->ifa_addr->sa_family;
+if (family == AF_INET || family == AF_INET6) {
+if (!strncmp(ifa->ifa_name, dev, IFNAMSIZ)) {
+cnt++;
+}
}
}
}
@@ -1901,7 +1903,7 @@ netdev_get_addrs(const char dev[], struct in6_addr
**paddr,
for (ifa = if_addr_list; ifa; ifa = ifa->ifa_next) {
int family;
-if (strncmp(ifa->ifa_name, dev, IFNAMSIZ)) {
+if (strncmp(ifa->ifa_name, dev, IFNAMSIZ) || ifa->ifa_addr == NULL) {
continue;
}
--
2.5.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] tunneling: Fix for concomitant IPv4 and IPv6 tunnels
When using an IPv6 tunnel on the same bridge as an IPv4 tunnel, the flow
received from the IPv6 tunnel would have an IPv4 address added to it, causing
problems when trying to put or execute the action on Linux datapath.
Clearing the IPv6 address when we have a valid IPv4 address fixes this problem.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
ofproto/tunnel.c | 4
tests/tunnel.at | 27 +++
2 files changed, 31 insertions(+)
diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
index 7430994..18297b2 100644
--- a/ofproto/tunnel.c
+++ b/ofproto/tunnel.c
@@ -419,12 +419,16 @@ tnl_port_send(const struct ofport_dpif *ofport, struct
flow *flow,
flow->tunnel.ip_src =
in6_addr_get_mapped_ipv4(&tnl_port->match.ipv6_src);
if (!flow->tunnel.ip_src) {
flow->tunnel.ipv6_src = tnl_port->match.ipv6_src;
+} else {
+flow->tunnel.ipv6_src = in6addr_any;
}
}
if (!cfg->ip_dst_flow) {
flow->tunnel.ip_dst =
in6_addr_get_mapped_ipv4(&tnl_port->match.ipv6_dst);
if (!flow->tunnel.ip_dst) {
flow->tunnel.ipv6_dst = tnl_port->match.ipv6_dst;
+} else {
+flow->tunnel.ipv6_dst = in6addr_any;
}
}
flow->pkt_mark = tnl_port->match.pkt_mark;
diff --git a/tests/tunnel.at b/tests/tunnel.at
index 0c033da..9f17194 100644
--- a/tests/tunnel.at
+++ b/tests/tunnel.at
@@ -524,3 +524,30 @@ Datapath actions:
set(tunnel(tun_id=0x0,dst=1.1.1.1,ttl=64,geneve({class=0x,
OVS_VSWITCHD_STOP
AT_CLEANUP
+
+AT_SETUP([tunnel - concomitant IPv6 and IPv4 tunnels])
+OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=vxlan \
+options:remote_ip=1.1.1.1 ofport_request=1 \
+-- add-port br0 p2 -- set Interface p2 type=vxlan \
+options:remote_ip=2001:cafe::1 ofport_request=2])
+AT_DATA([flows.txt], [dnl
+in_port=1,actions=2
+in_port=2,actions=1
+])
+OVS_VSWITCHD_DISABLE_TUNNEL_PUSH_POP
+
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy
'tunnel(tun_id=0,src=1.1.1.1,dst=1.1.1.2,ttl=64),in_port(4789)'], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+ [Datapath actions:
set(tunnel(tun_id=0x0,ipv6_dst=2001:cafe::1,ttl=64,flags(df|key))),4789
+])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy
'tunnel(tun_id=0x0,ipv6_src=2001:cafe::1,ipv6_dst=2001:cafe::2,ttl=64),in_port(4789)'],
[0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+ [Datapath actions:
set(tunnel(tun_id=0x0,dst=1.1.1.1,ttl=64,flags(df|key))),4789
+])
+
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
--
2.5.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [RFC PATCH] create vxlan device using rtnetlink interface
Hi, this is an RFC patch (could probably be 2 patches - more below), that
creates VXLAN devices in userspace and adds them as netdev vports, instead of
using the vxlan vport code.
The reason for this change is that it has been mentioned in the past that some
of the vport code should be considered compat code, that is, it won't receive
new features or flags.
That means tunnel devices should be created in userspace and added as netdev
vports. Some problems are noticeable in this patch, and I have gone through some
iterations back and forth.
First is the creation of the device under a switch/case. I tried to make this a
netdev_class function, overriding the netdev class by dpif_port_open_type. That
required exporting a lot of the vport functions. I can still do it that way, if
that's preferrable, but this seems more simple.
The other problem is during port_del, that since we don't have a netdev, the
dpif_port type would not be vxlan, and deciding whether to remove it or not
could not be made. In fact, this is one of the main reasons why this is an RFC.
The solution here is to decide on the type by the device's name, and there is
even a function for this, though it looks up for the netdev's already created,
those that probably come from the database. However, when OVS starts, it removes
ports from the datapath, and that information is not available, and may not even
be. In fact, since the dpif_port has a different name because it's a vport, it
won't match any netdev at all. So, I did the opposite of getting the type from
the dpif_port name, ie, if it contains vxlan_sys, it's of vxlan type. Even if we
make this more strict and use the port, we could still be in conflict with a
vxlan device created by the user with such name. This should have been one patch
by itself.
Since we already have similar problems, like failing to port_add when there is a
device named vxlan_sys_ created by the user, one could argue that this
patch does not introduce a new problem.
Jiri has suggested that we require users to create the interfaces themselves, by
using whatever method their OS has, and add them as netdev devices. That would
still require fetching some of the configuration from the device in order to
make it properly work with flow-based tunnels. In fact, if we set the remote or
local IP addresses on those devices, this would require multiple interfaces
instead of only one just to be able to specify the same level of configuration
as ovsdb allows us to. And that still requires some proper changes in order to
support flow-based tunnels (calling tnl_port_add/tnl_port_del with proper
configuration, for example).
Another option instead of relying on the device names is to use ifalias. It's a
user settable name that is opaque to the kernel and tools like iproute and
net-tools.
Any opinions on these options or other comments on the patch?
---
lib/dpif-netlink.c | 285 +++--
lib/netdev-vport.c | 47 ++---
lib/netdev-vport.h | 1 +
lib/netdev.c | 7 ++
4 files changed, 273 insertions(+), 67 deletions(-)
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 015ba20..6be6f52 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -780,10 +781,8 @@ get_vport_type(const struct dpif_netlink_vport *vport)
}
static enum ovs_vport_type
-netdev_to_ovs_vport_type(const struct netdev *netdev)
+netdev_to_ovs_vport_type(const char *type)
{
-const char *type = netdev_get_type(netdev);
-
if (!strcmp(type, "tap") || !strcmp(type, "system")) {
return OVS_VPORT_TYPE_NETDEV;
} else if (!strcmp(type, "internal")) {
@@ -804,19 +803,14 @@ netdev_to_ovs_vport_type(const struct netdev *netdev)
}
static int
-dpif_netlink_port_add__(struct dpif_netlink *dpif, struct netdev *netdev,
+dpif_netlink_port_add__(struct dpif_netlink *dpif, const char *name,
+enum ovs_vport_type type,
+struct ofpbuf *options,
odp_port_t *port_nop)
OVS_REQ_WRLOCK(dpif->upcall_lock)
{
-const struct netdev_tunnel_config *tnl_cfg;
-char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
-const char *name = netdev_vport_get_dpif_port(netdev,
- namebuf, sizeof namebuf);
-const char *type = netdev_get_type(netdev);
struct dpif_netlink_vport request, reply;
struct ofpbuf *buf;
-uint64_t options_stub[64 / 8];
-struct ofpbuf options;
struct nl_sock **socksp = NULL;
uint32_t *upcall_pids;
int error = 0;
@@ -831,52 +825,19 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, struct
netdev *netdev,
dpif_netlink_vport_init(&request);
request.cmd = OVS_VPORT_CMD_NEW;
request.dp_ifindex = dpif->dp_ifindex;
-request.type = netdev_to_ovs_vport_type(netdev);
-if (request.type == OVS_VPORT_TYPE_UNSPEC) {
-
Re: [ovs-dev] [RFC PATCH] create vxlan device using rtnetlink interface
On Fri, Apr 15, 2016 at 08:36:51PM -0700, Jesse Gross wrote:
> On Fri, Apr 15, 2016 at 2:30 PM, Thadeu Lima de Souza Cascardo
> wrote:
> > Hi, this is an RFC patch (could probably be 2 patches - more below), that
> > creates VXLAN devices in userspace and adds them as netdev vports, instead
> > of
> > using the vxlan vport code.
> >
> > The reason for this change is that it has been mentioned in the past that
> > some
> > of the vport code should be considered compat code, that is, it won't
> > receive
> > new features or flags.
>
> Thanks for looking at this. I agree that this is definitely a
> necessary direction.
>
> > First is the creation of the device under a switch/case. I tried to make
> > this a
> > netdev_class function, overriding the netdev class by dpif_port_open_type.
> > That
> > required exporting a lot of the vport functions. I can still do it that
> > way, if
> > that's preferrable, but this seems more simple.
>
> Can you give some examples of what would need to be exported? It would
> be nice to just set the open function for each type but if it is
> really horrible we can live with the switch statement.
>
netdev_vport_{alloc, construct, destruct, dealloc}, get_tunnel_config,
set_tunnel_config, get_netdev_tunnel_config.
We could also do it the other way around, write this code in netdev-vport.c and
export one or two functions from netdev-linux or dpif-netlink, and the create
function per tunnel type.
> > The other problem is during port_del, that since we don't have a netdev, the
> > dpif_port type would not be vxlan, and deciding whether to remove it or not
> > could not be made. In fact, this is one of the main reasons why this is an
> > RFC.
> >
> > The solution here is to decide on the type by the device's name, and there
> > is
> > even a function for this, though it looks up for the netdev's already
> > created,
> > those that probably come from the database. However, when OVS starts, it
> > removes
> > ports from the datapath, and that information is not available, and may not
> > even
> > be. In fact, since the dpif_port has a different name because it's a vport,
> > it
> > won't match any netdev at all. So, I did the opposite of getting the type
> > from
> > the dpif_port name, ie, if it contains vxlan_sys, it's of vxlan type. Even
> > if we
> > make this more strict and use the port, we could still be in conflict with a
> > vxlan device created by the user with such name. This should have been one
> > patch
> > by itself.
>
> What about using the driver name exposed through ethtool? I believe
> that all of the tunnel and internal devices expose this in a
> consistent way - i.e. a VXLAN device can be queried and get back the
> string "vxlan". Any driver other than the ones that we recognize can
> be assumed to be OVS_VPORT_TYPE_NETDEV.
>
I don't see how this address the case when the user adds a vxlan interface
created by the system. Like:
ip link add vxlan_sys_4789 type vxlan id 10 remote 192.168.123.1 dstport 4789
ovs-vsctl add-port br0 vxlan_sys_4789
Its driver would be vxlan. That goes to vxlan0 too.
But... wait a minute. We don't support adding devices name as vxlan_sys*. Such
names are reserved. I think that means we could probably rely on the names.
> > Jiri has suggested that we require users to create the interfaces
> > themselves, by
> > using whatever method their OS has, and add them as netdev devices. That
> > would
> > still require fetching some of the configuration from the device in order to
> > make it properly work with flow-based tunnels. In fact, if we set the
> > remote or
> > local IP addresses on those devices, this would require multiple interfaces
> > instead of only one just to be able to specify the same level of
> > configuration
> > as ovsdb allows us to. And that still requires some proper changes in order
> > to
> > support flow-based tunnels (calling tnl_port_add/tnl_port_del with proper
> > configuration, for example).
>
> I'm not too excited about this. It seems like it would be a regression
> - currently OVSDB allows remote creation of tunnels, so it seems like
> this would break existing setups if it also requires users to
> explicitly create tunnel devices on the host ahead of time.
Agreed. That's a very good reason not to go this path. And realizing we already
reserve the names, I am more comfortable relying on only that.
>
> One comment on the patch itself - it's possible that the device that
> is being created might not support al
Re: [ovs-dev] [RFC PATCH] create vxlan device using rtnetlink interface
On Mon, Apr 18, 2016 at 12:36:31PM +0200, Jiri Benc wrote:
> On Mon, 18 Apr 2016 06:57:22 -0300, Thadeu Lima de Souza Cascardo wrote:
> > But... wait a minute. We don't support adding devices name as vxlan_sys*.
> > Such
> > names are reserved. I think that means we could probably rely on the names.
>
> They are not. You can easily create an interface named vxlan_sys_4789
> by 'ip link add'.
>
I mean by using ovs-vsctl add-port. At vswitchd/bridge.c:iface_do_create,
if (netdev_is_reserved_name(iface_cfg->name)) {
VLOG_WARN("could not create interface %s, name is reserved",
iface_cfg->name);
error = EINVAL;
goto error;
}
netdev_is_reserved_name checks for names starting with the dpif_port prefix from
vports. That includes vxlan_sys.
[root@devconf2 ~]# ovs-vsctl add-port br0 vxlan_sys_
ovs-vsctl: Error detected while setting up 'vxlan_sys_'. See ovs-vswitchd
log for details.
[root@devconf2 ~]# tail /var/log/openvswitch/ovs-vswitchd.log
2016-04-18T07:34:01.422Z|00032|vlog|INFO|opened log file
/var/log/openvswitch/ovs-vswitchd.log
2016-04-18T09:30:34.142Z|00033|bridge|WARN|could not create interface
vxlan_sys_, name is reserved
> > That should be taken care of. We would get EINVAL, and that's why I return
> > EOPNOTSUPP if that's the case. Then, the code falls back to the compat mode.
>
> We would not get EINVAL. The interface will be created, netlink will
> return success but the interface won't be in the metadata mode.
>
> The same applies to all other configuration, e.g. VXLAN-GPE. The
> netlink command will succeed, the interface will be created but it
> won't be GPE.
>
> It sucks completely, it's a big gap in netlink design but all my
> attempts to solve this were rejected upstream.
I will test that and add the verification code.
Thanks.
Cascardo.
>
> Jiri
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [RFC PATCH] create vxlan device using rtnetlink interface
On Tue, Apr 19, 2016 at 04:25:32PM -0700, Jesse Gross wrote:
> On Mon, Apr 18, 2016 at 2:57 AM, Thadeu Lima de Souza Cascardo
> wrote:
> > On Fri, Apr 15, 2016 at 08:36:51PM -0700, Jesse Gross wrote:
> >> On Fri, Apr 15, 2016 at 2:30 PM, Thadeu Lima de Souza Cascardo
> >> wrote:
> >> > Hi, this is an RFC patch (could probably be 2 patches - more below), that
> >> > creates VXLAN devices in userspace and adds them as netdev vports,
> >> > instead of
> >> > using the vxlan vport code.
> >> >
> >> > The reason for this change is that it has been mentioned in the past
> >> > that some
> >> > of the vport code should be considered compat code, that is, it won't
> >> > receive
> >> > new features or flags.
> >>
> >> Thanks for looking at this. I agree that this is definitely a
> >> necessary direction.
> >>
> >> > First is the creation of the device under a switch/case. I tried to make
> >> > this a
> >> > netdev_class function, overriding the netdev class by
> >> > dpif_port_open_type. That
> >> > required exporting a lot of the vport functions. I can still do it that
> >> > way, if
> >> > that's preferrable, but this seems more simple.
> >>
> >> Can you give some examples of what would need to be exported? It would
> >> be nice to just set the open function for each type but if it is
> >> really horrible we can live with the switch statement.
> >>
> >
> > netdev_vport_{alloc, construct, destruct, dealloc}, get_tunnel_config,
> > set_tunnel_config, get_netdev_tunnel_config.
> >
> > We could also do it the other way around, write this code in netdev-vport.c
> > and
> > export one or two functions from netdev-linux or dpif-netlink, and the
> > create
> > function per tunnel type.
>
> OK, thanks, I understand now. I think what you have currently is
> probably the best solution for the time being.
>
> >> > The other problem is during port_del, that since we don't have a netdev,
> >> > the
> >> > dpif_port type would not be vxlan, and deciding whether to remove it or
> >> > not
> >> > could not be made. In fact, this is one of the main reasons why this is
> >> > an RFC.
> >> >
> >> > The solution here is to decide on the type by the device's name, and
> >> > there is
> >> > even a function for this, though it looks up for the netdev's already
> >> > created,
> >> > those that probably come from the database. However, when OVS starts, it
> >> > removes
> >> > ports from the datapath, and that information is not available, and may
> >> > not even
> >> > be. In fact, since the dpif_port has a different name because it's a
> >> > vport, it
> >> > won't match any netdev at all. So, I did the opposite of getting the
> >> > type from
> >> > the dpif_port name, ie, if it contains vxlan_sys, it's of vxlan type.
> >> > Even if we
> >> > make this more strict and use the port, we could still be in conflict
> >> > with a
> >> > vxlan device created by the user with such name. This should have been
> >> > one patch
> >> > by itself.
> >>
> >> What about using the driver name exposed through ethtool? I believe
> >> that all of the tunnel and internal devices expose this in a
> >> consistent way - i.e. a VXLAN device can be queried and get back the
> >> string "vxlan". Any driver other than the ones that we recognize can
> >> be assumed to be OVS_VPORT_TYPE_NETDEV.
> >>
> >
> > I don't see how this address the case when the user adds a vxlan interface
> > created by the system. Like:
> >
> > ip link add vxlan_sys_4789 type vxlan id 10 remote 192.168.123.1 dstport
> > 4789
> > ovs-vsctl add-port br0 vxlan_sys_4789
> >
> > Its driver would be vxlan. That goes to vxlan0 too.
>
> I think we can check the combination of device type and the options
> that are set on it (the same as what we need to do after we create the
> device). If all of those match then it doesn't matter whether we added
> it previously or the user added it - the device will work exactly the
> same either way. If there isn't a match then we should bail out
> without adding the port. This is pretty similar to the behavior of the
> current compat code in t
Re: [ovs-dev] [RFC PATCH] create vxlan device using rtnetlink interface
On Wed, Apr 20, 2016 at 11:38:31AM -0700, Jesse Gross wrote: > Thanks for that analysis. I agree with you that this looks safe. I'm > glad - there are fewer corner cases than I was expecting. > > One minor comment that I noticed on the patch itself - I don't know if > the port mapping functions are handling IPsec variants of tunnels > correctly in all situations (or maybe are handling them by accident). > Since both IPsec and non-IPsec ports share the same dpif_port name, we > might get the wrong class back and then it seems like we don't handle > the IPsec class types when converting between names and types, which > could itself be an independent bug. Can you take a look? Sure, I am not too familiar with the ipsec code. Looking at it, I don't see anything really special about it. It just requires that some configuration is available and that the ovs-monitor-ipsec daemon is running. When adding and removing devices, the same procedure would still work. So, I don't see why we would need to handle it specially, do you? On the other hand, I noticed that VXLAN-GBP tunnels will have the problem of sharing the same dpif_port name with non GBP tunnels. That means that tunnels using different remote IPs with the same port all must be GBP or not GBP. If we used extensions for GPE, that would also apply there. Maybe we should just add an infix gbp and gpe, resulting in vxlan_sys_gbp_4789 and vxlan_sys_gpe_4789. How about that? Regards. Cascardo. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 07/15] lib: add format_in6_addr and scan_in6_addr
From: Jiri Benc
Add in6_addr counterparts to the existing format and scan functions.
Otherwise we'd need to recast all the time.
Signed-off-by: Jiri Benc
---
lib/odp-util.c | 28 ++--
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/lib/odp-util.c b/lib/odp-util.c
index e131e36..752e2ea 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -1758,13 +1758,12 @@ format_ipv4(struct ds *ds, const char *name, ovs_be32
key,
}
static void
-format_ipv6(struct ds *ds, const char *name, const ovs_be32 key_[4],
-const ovs_be32 (*mask_)[4], bool verbose)
+format_in6_addr(struct ds *ds, const char *name,
+const struct in6_addr *key,
+const struct in6_addr *mask,
+bool verbose)
{
char buf[INET6_ADDRSTRLEN];
-const struct in6_addr *key = (const struct in6_addr *)key_;
-const struct in6_addr *mask = mask_ ? (const struct in6_addr *)*mask_
-: NULL;
bool mask_empty = mask && ipv6_mask_is_any(mask);
if (verbose || !mask_empty) {
@@ -1781,6 +1780,16 @@ format_ipv6(struct ds *ds, const char *name, const
ovs_be32 key_[4],
}
static void
+format_ipv6(struct ds *ds, const char *name, const ovs_be32 key_[4],
+const ovs_be32 (*mask_)[4], bool verbose)
+{
+format_in6_addr(ds, name,
+(const struct in6_addr *)key_,
+mask_ ? (const struct in6_addr *)*mask_ : NULL,
+verbose);
+}
+
+static void
format_ipv6_label(struct ds *ds, const char *name, ovs_be32 key,
const ovs_be32 *mask, bool verbose)
{
@@ -2753,7 +2762,7 @@ scan_ipv4(const char *s, ovs_be32 *key, ovs_be32 *mask)
}
static int
-scan_ipv6(const char *s, ovs_be32 (*key)[4], ovs_be32 (*mask)[4])
+scan_in6_addr(const char *s, struct in6_addr *key, struct in6_addr *mask)
{
int n;
char ipv6_s[IPV6_SCAN_LEN + 1];
@@ -2776,6 +2785,13 @@ scan_ipv6(const char *s, ovs_be32 (*key)[4], ovs_be32
(*mask)[4])
}
static int
+scan_ipv6(const char *s, ovs_be32 (*key)[4], ovs_be32 (*mask)[4])
+{
+return scan_in6_addr(s, key ? (struct in6_addr *) *key : NULL,
+ mask ? (struct in6_addr *) *mask : NULL);
+}
+
+static int
scan_ipv6_label(const char *s, ovs_be32 *key, ovs_be32 *mask)
{
int key_, mask_;
--
2.4.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 04/15] tunneling: extend tnl_match with ipv6
From: Jiri Benc
[cascardo: use IPv4-mapped IPv6 addresses]
Signed-off-by: Jiri Benc
Signed-off-by: Thadeu Lima de Souza Cascardo
Co-authored-by: Thadeu Lima de Souza Cascardo
---
ofproto/tunnel.c | 36 ++--
1 file changed, 22 insertions(+), 14 deletions(-)
diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
index b85c2b5..60e4773 100644
--- a/ofproto/tunnel.c
+++ b/ofproto/tunnel.c
@@ -47,8 +47,8 @@ VLOG_DEFINE_THIS_MODULE(tunnel);
struct tnl_match {
ovs_be64 in_key;
-ovs_be32 ip_src;
-ovs_be32 ip_dst;
+struct in6_addr ipv6_src;
+struct in6_addr ipv6_dst;
odp_port_t odp_port;
uint32_t pkt_mark;
bool in_key_flow;
@@ -161,8 +161,12 @@ tnl_port_add__(const struct ofport_dpif *ofport, const
struct netdev *netdev,
tnl_port->change_seq = netdev_get_change_seq(tnl_port->netdev);
tnl_port->match.in_key = cfg->in_key;
-tnl_port->match.ip_src = cfg->ip_src;
-tnl_port->match.ip_dst = cfg->ip_dst;
+if (cfg->ip_src) {
+in6_addr_set_mapped_ipv4(&tnl_port->match.ipv6_src, cfg->ip_src);
+}
+if (cfg->ip_dst) {
+in6_addr_set_mapped_ipv4(&tnl_port->match.ipv6_dst, cfg->ip_dst);
+}
tnl_port->match.ip_src_flow = cfg->ip_src_flow;
tnl_port->match.ip_dst_flow = cfg->ip_dst_flow;
tnl_port->match.pkt_mark = cfg->ipsec ? IPSEC_MARK : 0;
@@ -412,10 +416,10 @@ tnl_port_send(const struct ofport_dpif *ofport, struct
flow *flow,
}
if (!cfg->ip_src_flow) {
-flow->tunnel.ip_src = tnl_port->match.ip_src;
+flow->tunnel.ip_src =
in6_addr_get_mapped_ipv4(&tnl_port->match.ipv6_src);
}
if (!cfg->ip_dst_flow) {
-flow->tunnel.ip_dst = tnl_port->match.ip_dst;
+flow->tunnel.ip_dst =
in6_addr_get_mapped_ipv4(&tnl_port->match.ipv6_dst);
}
flow->pkt_mark = tnl_port->match.pkt_mark;
@@ -535,10 +539,12 @@ tnl_find(const struct flow *flow) OVS_REQ_RDLOCK(rwlock)
* here as a description of how to treat received
* packets. */
match.in_key = in_key_flow ? 0 : flow->tunnel.tun_id;
-match.ip_src = (ip_src == IP_SRC_CFG
-? flow->tunnel.ip_dst
-: 0);
-match.ip_dst = ip_dst_flow ? 0 : flow->tunnel.ip_src;
+if (ip_src == IP_SRC_CFG && flow->tunnel.ip_dst) {
+in6_addr_set_mapped_ipv4(&match.ipv6_src,
flow->tunnel.ip_dst);
+}
+if (!ip_dst_flow && flow->tunnel.ip_src) {
+in6_addr_set_mapped_ipv4(&match.ipv6_dst,
flow->tunnel.ip_src);
+}
match.odp_port = flow->in_port.odp_port;
match.pkt_mark = flow->pkt_mark;
match.in_key_flow = in_key_flow;
@@ -567,7 +573,7 @@ tnl_match_map(const struct tnl_match *m)
enum ip_src_type ip_src;
ip_src = (m->ip_src_flow ? IP_SRC_FLOW
- : m->ip_src ? IP_SRC_CFG
+ : ipv6_addr_is_set(&m->ipv6_src) ? IP_SRC_CFG
: IP_SRC_ANY);
return &tnl_match_maps[6 * m->in_key_flow + 3 * m->ip_dst_flow + ip_src];
@@ -578,10 +584,12 @@ tnl_match_fmt(const struct tnl_match *match, struct ds
*ds)
OVS_REQ_RDLOCK(rwlock)
{
if (!match->ip_dst_flow) {
-ds_put_format(ds, IP_FMT"->"IP_FMT, IP_ARGS(match->ip_src),
- IP_ARGS(match->ip_dst));
+print_ipv6_mapped(ds, &match->ipv6_src);
+ds_put_cstr(ds, "->");
+print_ipv6_mapped(ds, &match->ipv6_dst);
} else if (!match->ip_src_flow) {
-ds_put_format(ds, IP_FMT"->flow", IP_ARGS(match->ip_src));
+print_ipv6_mapped(ds, &match->ipv6_src);
+ds_put_cstr(ds, "->flow");
} else {
ds_put_cstr(ds, "flow->flow");
}
--
2.4.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 03/15] tnl-arp-cache: Include tnl-arp-cache.h as first header.
Signed-off-by: Thadeu Lima de Souza Cascardo --- lib/tnl-arp-cache.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/tnl-arp-cache.c b/lib/tnl-arp-cache.c index 85de312..d817d66 100644 --- a/lib/tnl-arp-cache.c +++ b/lib/tnl-arp-cache.c @@ -15,6 +15,9 @@ */ #include + +#include "tnl-arp-cache.h" + #include #include #include @@ -33,7 +36,6 @@ #include "seq.h" #include "socket-util.h" #include "timeval.h" -#include "tnl-arp-cache.h" #include "unaligned.h" #include "unixctl.h" #include "util.h" -- 2.4.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 01/15] lib: add ipv6 helper functions for tnl_config
From: Jiri Benc
These functions will be used by the next patches.
Signed-off-by: Jiri Benc
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/smap.c| 11 +++
lib/smap.h| 1 +
lib/socket-util.c | 9 +
lib/socket-util.h | 1 +
4 files changed, 22 insertions(+)
diff --git a/lib/smap.c b/lib/smap.c
index 1d59f56..2c41013 100644
--- a/lib/smap.c
+++ b/lib/smap.c
@@ -95,6 +95,17 @@ smap_add_format(struct smap *smap, const char *key, const
char *format, ...)
hash_bytes(key, key_len, 0));
}
+/* Adds 'key' paired with a string representation of 'addr'. It is the
+ * caller's responsibility to avoid duplicate keys if desirable. */
+void
+smap_add_ipv6(struct smap *smap, const char *key, struct in6_addr *addr)
+{
+char buf[INET6_ADDRSTRLEN];
+
+inet_ntop(AF_INET6, addr, buf, sizeof buf);
+smap_add(smap, key, buf);
+}
+
/* Searches for 'key' in 'smap'. If it does not already exists, adds it.
* Otherwise, changes its value to 'value'. */
void
diff --git a/lib/smap.h b/lib/smap.h
index 3bfbb71..489497a 100644
--- a/lib/smap.h
+++ b/lib/smap.h
@@ -66,6 +66,7 @@ struct smap_node *smap_add_nocopy(struct smap *, char *, char
*);
bool smap_add_once(struct smap *, const char *, const char *);
void smap_add_format(struct smap *, const char *key, const char *, ...)
OVS_PRINTF_FORMAT(3, 4);
+void smap_add_ipv6(struct smap *, const char *, struct in6_addr *);
void smap_replace(struct smap *, const char *, const char *);
void smap_remove(struct smap *, const char *);
diff --git a/lib/socket-util.c b/lib/socket-util.c
index 206e17b..97ee01b 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -136,6 +136,15 @@ set_dscp(int fd, int family, uint8_t dscp)
return retval ? sock_errno() : 0;
}
+/* Checks whether 'host_name' is an IPv4 or IPv6 address. It is assumed
+ * that 'host_name' is valid. Returns false if it is IPv4 address, true if
+ * it is IPv6 address. */
+bool
+addr_is_ipv6(const char *host_name)
+{
+return strchr(host_name, ':') != NULL;
+}
+
/* Translates 'host_name', which must be a string representation of an IP
* address, into a numeric IP address in '*addr'. Returns 0 if successful,
* otherwise a positive errno value. */
diff --git a/lib/socket-util.h b/lib/socket-util.h
index 8015c7f..c3c1224 100644
--- a/lib/socket-util.h
+++ b/lib/socket-util.h
@@ -32,6 +32,7 @@ void xset_nonblocking(int fd);
void setsockopt_tcp_nodelay(int fd);
int set_dscp(int fd, int family, uint8_t dscp);
+bool addr_is_ipv6(const char *host_name);
int lookup_ip(const char *host_name, struct in_addr *address);
int lookup_ipv6(const char *host_name, struct in6_addr *address);
--
2.4.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 14/15] Allow flow-based IPv6 tunnels to be configured with OpenFlow
With this patch, it is possible to set the IPv6 source and destination address
in flow-based tunnels.
$ ovs-ofctl add-flow br0 "in_port=LOCAL
actions=set_field:2001:cafe::92->tun_ipv6_dst"
Signed-off-by: Thadeu Lima de Souza Cascardo
Signed-off-by: Jiri Benc
Co-authored-by: Jiri Benc
---
lib/meta-flow.c | 42 ++
lib/meta-flow.h | 35 +++
lib/nx-match.c | 4
tests/ofproto.at | 4 +++-
4 files changed, 84 insertions(+), 1 deletion(-)
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index eaf4ca8..bfc0ab2 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -190,6 +190,10 @@ mf_is_all_wild(const struct mf_field *mf, const struct
flow_wildcards *wc)
return !wc->masks.tunnel.ip_src;
case MFF_TUN_DST:
return !wc->masks.tunnel.ip_dst;
+case MFF_TUN_IPV6_SRC:
+return ipv6_mask_is_any(&wc->masks.tunnel.ipv6_src);
+case MFF_TUN_IPV6_DST:
+return ipv6_mask_is_any(&wc->masks.tunnel.ipv6_dst);
case MFF_TUN_ID:
return !wc->masks.tunnel.tun_id;
case MFF_TUN_TOS:
@@ -496,6 +500,8 @@ mf_is_value_valid(const struct mf_field *mf, const union
mf_value *value)
case MFF_TUN_ID:
case MFF_TUN_SRC:
case MFF_TUN_DST:
+case MFF_TUN_IPV6_SRC:
+case MFF_TUN_IPV6_DST:
case MFF_TUN_TOS:
case MFF_TUN_TTL:
case MFF_TUN_GBP_ID:
@@ -617,6 +623,12 @@ mf_get_value(const struct mf_field *mf, const struct flow
*flow,
case MFF_TUN_DST:
value->be32 = flow->tunnel.ip_dst;
break;
+case MFF_TUN_IPV6_SRC:
+value->ipv6 = flow->tunnel.ipv6_src;
+break;
+case MFF_TUN_IPV6_DST:
+value->ipv6 = flow->tunnel.ipv6_dst;
+break;
case MFF_TUN_FLAGS:
value->be16 = htons(flow->tunnel.flags & FLOW_TNL_PUB_F_MASK);
break;
@@ -858,6 +870,12 @@ mf_set_value(const struct mf_field *mf,
case MFF_TUN_DST:
match_set_tun_dst(match, value->be32);
break;
+case MFF_TUN_IPV6_SRC:
+match_set_tun_ipv6_src(match, &value->ipv6);
+break;
+case MFF_TUN_IPV6_DST:
+match_set_tun_ipv6_dst(match, &value->ipv6);
+break;
case MFF_TUN_FLAGS:
match_set_tun_flags(match, ntohs(value->be16));
break;
@@ -1168,6 +1186,12 @@ mf_set_flow_value(const struct mf_field *mf,
case MFF_TUN_DST:
flow->tunnel.ip_dst = value->be32;
break;
+case MFF_TUN_IPV6_SRC:
+flow->tunnel.ipv6_src = value->ipv6;
+break;
+case MFF_TUN_IPV6_DST:
+flow->tunnel.ipv6_dst = value->ipv6;
+break;
case MFF_TUN_FLAGS:
flow->tunnel.flags = (flow->tunnel.flags & ~FLOW_TNL_PUB_F_MASK) |
ntohs(value->be16);
@@ -1472,6 +1496,18 @@ mf_set_wild(const struct mf_field *mf, struct match
*match, char **err_str)
case MFF_TUN_DST:
match_set_tun_dst_masked(match, htonl(0), htonl(0));
break;
+case MFF_TUN_IPV6_SRC:
+memset(&match->wc.masks.tunnel.ipv6_src, 0,
+ sizeof match->wc.masks.tunnel.ipv6_src);
+memset(&match->flow.tunnel.ipv6_src, 0,
+ sizeof match->flow.tunnel.ipv6_src);
+break;
+case MFF_TUN_IPV6_DST:
+memset(&match->wc.masks.tunnel.ipv6_dst, 0,
+ sizeof match->wc.masks.tunnel.ipv6_dst);
+memset(&match->flow.tunnel.ipv6_dst, 0,
+ sizeof match->flow.tunnel.ipv6_dst);
+break;
case MFF_TUN_FLAGS:
match_set_tun_flags_masked(match, 0, 0);
break;
@@ -1764,6 +1800,12 @@ mf_set(const struct mf_field *mf,
case MFF_TUN_DST:
match_set_tun_dst_masked(match, value->be32, mask->be32);
break;
+case MFF_TUN_IPV6_SRC:
+match_set_tun_ipv6_src_masked(match, &value->ipv6, &mask->ipv6);
+break;
+case MFF_TUN_IPV6_DST:
+match_set_tun_ipv6_dst_masked(match, &value->ipv6, &mask->ipv6);
+break;
case MFF_TUN_FLAGS:
match_set_tun_flags_masked(match, ntohs(value->be16),
ntohs(mask->be16));
break;
diff --git a/lib/meta-flow.h b/lib/meta-flow.h
index d22c156..5f08049 100644
--- a/lib/meta-flow.h
+++ b/lib/meta-flow.h
@@ -368,6 +368,41 @@ enum OVS_PACKED_ENUM mf_field_id {
*/
MFF_TUN_DST,
+/* "tun_ipv6_src".
+ *
+ * The IPv6 source address in the outer IP header of a tunneled packet.
+ *
+ * For non-tunneled packets, the value is 0.
+ *
+ * Type: be128.
+ * Maskable: bitwise.
+ * Formatting: IPv6.
+ * Prerequisites: none.
+ * Access: read/write.
+ * NXM: NXM_NX_TUN_IPV6_SRC(109) since v2.5.
+ * OXM: none.
+ * Prefix lookup member: tunnel.ipv6_src.
+ */
+MFF_TUN_IPV6_SRC,
+
+
[ovs-dev] [PATCH 02/15] tnl-arp-cache: fix log error when using tnl/arp/set with IPv6
lookup_ip will emit an error when used with an IPv6 address, like below.
2015-10-20T18:48:22.357Z|00036|socket_util|ERR|"2001:cafe::92" is not a valid
IP address
Verify if address looks like IPv6 before giving it to either lookup_ip or
lookup_ipv6.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/tnl-arp-cache.c | 22 ++
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/lib/tnl-arp-cache.c b/lib/tnl-arp-cache.c
index d456d0c..85de312 100644
--- a/lib/tnl-arp-cache.c
+++ b/lib/tnl-arp-cache.c
@@ -236,18 +236,32 @@ tnl_arp_cache_flush(struct unixctl_conn *conn, int argc
OVS_UNUSED,
unixctl_command_reply(conn, "OK");
}
+static int
+lookup_any(const char *host_name, struct in6_addr *address)
+{
+if (addr_is_ipv6(host_name)) {
+return lookup_ipv6(host_name, address);
+} else {
+int r;
+struct in_addr ip;
+r = lookup_ip(host_name, &ip);
+if (r == 0) {
+in6_addr_set_mapped_ipv4(address, ip.s_addr);
+}
+return r;
+}
+return ENOENT;
+}
+
static void
tnl_arp_cache_add(struct unixctl_conn *conn, int argc OVS_UNUSED,
const char *argv[], void *aux OVS_UNUSED)
{
const char *br_name = argv[1];
struct eth_addr mac;
-struct in_addr ip;
struct in6_addr ip6;
-if (lookup_ip(argv[2], &ip) == 0) {
-in6_addr_set_mapped_ipv4(&ip6, ip.s_addr);
-} else if (lookup_ipv6(argv[2], &ip6) != 0) {
+if (lookup_any(argv[2], &ip6) != 0) {
unixctl_command_reply_error(conn, "bad IP address");
return;
}
--
2.4.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OVS bridges always report UP + hotplugging support?
On Wed, Nov 04, 2015 at 02:06:56PM -0800, Karol Mroz wrote: > Hello, > > Support for configuring OVS bridges has been added to our (SUSE) network > manager. > During test (OVS 2.1.2, kernel 3.12.x), a couple things have been uncovered > that > I'd love to get some feedback on. > > 1. OVS bridges appear to report up state, regardless if this is true. For > example, > up is reported even if the underlying bridge port is down, or internally > OVS has > not yet set up the necessary flows to allow for data transmission. Is this > something > that is planned to be addressed? I realize 2.1.2 is an older version, so > perhaps > this has already been addressed in later versions of OVS? > > 2. OVS does not appear to handle hotplugging. For example, if a port, which > is attached > to the an OVS bridge (thus is correctly reflected in ovsdb) arrives on the > system _after_ > the actual `add-port` operation was performed, it will not get enslaved > under the ovs-system > interface. In this case, a manual `del-port/add-port` is needed. Is there > any plan to > add netlink monitoring of interface states (NEWLINK/DELLINK, etc) + > enslavement > when appropriate? > This has been implemented. Check commit e21c6643a02c6b446d2fbdfde366ea303b4c2730, which is not present in any release yet. Hope that helps. Cascardo. > Thanks! > Karol > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovs-router: Report ovs/route/add errors as errors.
On Fri, Nov 06, 2015 at 03:08:29PM -0800, Ben Pfaff wrote:
> The _error version should be used to report errors.
>
> Signed-off-by: Ben Pfaff
> ---
> lib/ovs-router.c | 11 ++-
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/lib/ovs-router.c b/lib/ovs-router.c
> index 2f093e8..50737a2 100644
> --- a/lib/ovs-router.c
> +++ b/lib/ovs-router.c
> @@ -275,7 +275,8 @@ ovs_router_add(struct unixctl_conn *conn, int argc,
> gw6 = in6addr_any;
> }
> } else {
> -unixctl_command_reply(conn, "Invalid parameters");
> +unixctl_command_reply_error(conn, "Invalid parameters");
> +return;
> }
> ovs_router_insert__(plen + 32, &ip6, plen, argv[2], &gw6);
> unixctl_command_reply(conn, "OK");
> @@ -293,13 +294,13 @@ ovs_router_del(struct unixctl_conn *conn, int argc
> OVS_UNUSED,
> in6_addr_set_mapped_ipv4(&ip6, ip);
> plen += 96;
> } else if (!scan_ipv6_route(argv[1], &ip6, &plen)) {
> -unixctl_command_reply(conn, "Invalid parameters");
> +unixctl_command_reply_error(conn, "Invalid parameters");
> }
> if (rt_entry_delete(plen + 32, &ip6, plen)) {
> unixctl_command_reply(conn, "OK");
> seq_change(tnl_conf_seq);
> } else {
> -unixctl_command_reply(conn, "Not found");
> +unixctl_command_reply_error(conn, "Not found");
> }
> }
>
> @@ -347,7 +348,7 @@ ovs_router_lookup_cmd(struct unixctl_conn *conn, int argc
> OVS_UNUSED,
> if (scan_ipv4_route(argv[1], &ip, &plen) && plen == 32) {
> in6_addr_set_mapped_ipv4(&ip6, ip);
> } else if (!(scan_ipv6_route(argv[1], &ip6, &plen) && plen == 128)) {
> -unixctl_command_reply(conn, "Invalid parameters");
> +unixctl_command_reply_error(conn, "Invalid parameters");
> }
This should have a return here as well. Otherwise, we may query for an invalid
address, and would assert at the second unixctl_command_reply.
Cascardo.
>
> if (ovs_router_lookup(&ip6, iface, &gw)) {
> @@ -358,7 +359,7 @@ ovs_router_lookup_cmd(struct unixctl_conn *conn, int argc
> OVS_UNUSED,
> unixctl_command_reply(conn, ds_cstr(&ds));
> ds_destroy(&ds);
> } else {
> -unixctl_command_reply(conn, "Not found");
> +unixctl_command_reply_error(conn, "Not found");
> }
> }
>
> --
> 2.1.3
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 05/15] tunneling: add IPv6 support to netdev_tunnel_config
On Tue, Nov 10, 2015 at 02:24:06PM -0800, Ben Pfaff wrote:
> On Thu, Oct 22, 2015 at 03:28:58PM -0200, Thadeu Lima de Souza Cascardo wrote:
> > From: Jiri Benc
> >
> > Allow configuration of IPv6 tunnel endpoints.
> >
> > [cascardo: removed support for netlink datapath configuration]
> > [cascardo: use IPv4 mapped IPv6 addresses]
> > [cascardo: use only flow, instead of flow and flow6]
> >
> > Signed-off-by: Jiri Benc
> > Signed-off-by: Thadeu Lima de Souza Cascardo
> > Co-authored-by: Thadeu Lima de Souza Cascardo
>
> I suggest folding this in, for style and to avoid a cast:
>
> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
> index cf393fa..2500996 100644
> --- a/lib/netdev-vport.c
> +++ b/lib/netdev-vport.c
> @@ -428,24 +428,28 @@ static int
> parse_tunnel_ip(const char *value, bool accept_mcast, bool *flow,
> struct in6_addr *ipv6, uint16_t *protocol)
> {
> -ovs_be32 ip;
> if (!strcmp(value, "flow")) {
> *flow = true;
> *protocol = 0;
> return 0;
> }
> if (addr_is_ipv6(value)) {
> -if (lookup_ipv6(value, ipv6))
> -return ENOENT;
> -if (!accept_mcast && ipv6_addr_is_multicast(ipv6))
> -return EINVAL;
> +if (lookup_ipv6(value, ipv6)) {
> +return ENOENT;
> +}
> +if (!accept_mcast && ipv6_addr_is_multicast(ipv6)){
> +return EINVAL;
> +}
> *protocol = ETH_TYPE_IPV6;
> } else {
> -if (lookup_ip(value, (struct in_addr *)&ip))
> -return ENOENT;
> -if (!accept_mcast && ip_is_multicast(ip))
> -return EINVAL;
> -in6_addr_set_mapped_ipv4(ipv6, ip);
> +struct in_addr ip;
> +if (lookup_ip(value, &ip)) {
> +return ENOENT;
> +}
> +if (!accept_mcast && ip_is_multicast(ip.s_addr)) {
> +return EINVAL;
> +}
> +in6_addr_set_mapped_ipv4(ipv6, ip.s_addr);
> *protocol = ETH_TYPE_IP;
> }
> return 0;
>
> The code in set_tunnel_config() makes me think that smap_add_ipv6()
> itself should check whether the address is IPv4-mapped and just use an
> IPv4 address in that case.
Thanks for all the reviews, Ben. I appreciate it a lot.
I will fold that in, and take a look at set_tunnel_config.
Thanks.
Cascardo.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 08/15] datapath: partial backport "netlink attributes for IPv6 tunneling"
On Tue, Nov 10, 2015 at 02:31:13PM -0800, Ben Pfaff wrote:
> On Thu, Oct 22, 2015 at 03:29:01PM -0200, Thadeu Lima de Souza Cascardo wrote:
> > Add netlink attributes for IPv6 tunnel addresses. This enables IPv6 support
> > for tunnels.
> >
> > [cascardo: Backport the key attribute values only.]
> >
> > Signed-off-by: Thadeu Lima de Souza Cascardo
>
> This causes a new warning:
>
> ../lib/odp-util.c:2116:17: error: enumeration values
> 'OVS_TUNNEL_KEY_ATTR_IPV6_SRC' and 'OVS_TUNNEL_KEY_ATTR_IPV6_DST' not
> explicitly handled in switch [-Werror,-Wswitch-enum]
> switch (type) {
> ^
>
> (I'm the wrong person to review a datapath change anyway.)
I will look at the warning, didn't show up for me when I used sparse and clang.
I need to check why that have slipped. Nonetheless, it would be nice to hear
from Pravin and Joe if the backport like this is OK.
Thanks for your review.
Cascardo.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 09/15] tunneling: extend flow_tnl with ipv6 addresses
On Tue, Nov 10, 2015 at 02:34:02PM -0800, Ben Pfaff wrote: > On Thu, Oct 22, 2015 at 03:29:02PM -0200, Thadeu Lima de Souza Cascardo wrote: > > From: Jiri Benc > > > > Note that because there's been no prerequisite on the outer protocol, > > we cannot add it now. Instead, treat the ipv4 and ipv6 dst fields in the way > > that either both are null, or at most one of them is non-null. > > > > [cascardo: abstract testing either dst with flow_tnl_dst_is_set] > > > > Signed-off-by: Jiri Benc > > Signed-off-by: Thadeu Lima de Souza Cascardo > > Co-authored-by: Thadeu Lima de Souza Cascardo > > One wonders whether it would be worth merging the ip_dst and ipv6_dst > fields in struct flow_tnl, using IPv6-mapped IPv4 addresses for the IPv4 > case. Did you consider that? Yes, I tried that, and had to go back to the way Jiri implemented it. The main problem was meta-flow and match. I tried using a union, so meta-flow.inc auto generated code would still work. However, when setting IPv4 tunnel addresses, the prefix would be all zeroes, not 0:0:0:0:0:0:::. I didn't manage to make that work. Maybe now that I have put my head a little more into meta-flow and match code, I may find a way to do that. I will take a new look and see if I can come back with new code or a better commit message, explaining why that would not be the best course in this case. When I went back to the original implementation, I knew an explanation about not using IPv4-mapped addresses was due, but I must have forgotten after working on the testsuite and many rebases. Cascardo. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 10/15] Increase FLOW_WC_SEQ
On Tue, Nov 10, 2015 at 04:00:03PM -0800, Ben Pfaff wrote: > On Thu, Oct 22, 2015 at 03:29:03PM -0200, Thadeu Lima de Souza Cascardo wrote: > > From: Jiri Benc > > > > The flow structure was changed, increase the sequence number. All the > > asserts either do not apply or have been resolved by previous patches in > > this set. > > > > Signed-off-by: Jiri Benc > > I recommend squashing this with the previous patch, because that's where > the benefit to review to comes in: the reviewer can see each place where > the FLOW_WC_SEQ assertion had to change and then see whether the > corresponding change, if any, next to that update was appropriate. Will do. I noticed most changes there were done like that, but I kept how Jiri did it. Cascardo. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 11/15] ofproto-dpif-xlate: Add IPv6 ND support for XC_TNL_ARP
On Tue, Nov 10, 2015 at 04:03:13PM -0800, Ben Pfaff wrote:
> On Thu, Oct 22, 2015 at 03:29:04PM -0200, Thadeu Lima de Souza Cascardo wrote:
> > Use IPv4-mapped addresses and use either tnl_arp_lookup or tnl_nd_lookup.
> >
> > Signed-off-by: Thadeu Lima de Souza Cascardo
>
> ...
>
> > @@ -5348,8 +5349,13 @@ xlate_push_stats(struct xlate_cache *xcache,
> > break;
> > case XC_TNL_ARP:
> > /* Lookup arp to avoid arp timeout. */
> > -tnl_arp_lookup(entry->u.tnl_arp_cache.br_name,
> > - entry->u.tnl_arp_cache.d_ip, &dmac);
> > +d_ip =
> > in6_addr_get_mapped_ipv4(&entry->u.tnl_arp_cache.d_ipv6);
> > +if (d_ip) {
> > +tnl_arp_lookup(entry->u.tnl_arp_cache.br_name, d_ip,
> > &dmac);
> > +} else {
> > +tnl_nd_lookup(entry->u.tnl_arp_cache.br_name,
> > + &entry->u.tnl_arp_cache.d_ipv6, &dmac);
> > +}
>
> This code seems a little silly to me, because it is going to some
> trouble to distinguish IPv4 from IPv6 and pick the correct
> tnl_*_lookup() function, and either function it picks is going to
> convert that right back to do the lookup. I think it would be more
> sensible to export tnl_arp_lookup__() so that the double conversion
> isn't needed.
>
> Thanks,
>
> Ben.
Urgh. Well, I could just use tnl_nd_lookup here with an extra comment about
that. Exporting tnl_arp_lookup__ would require exporting struct tnl_arp_entry.
But I see your point. The only bad point for me in using tnl_nd_lookup directly
would be the naming. But since we already have an IPv6 address in our hands, why
not just add a comment assuring this will work on IPv4-mapped addresses as well?
I will send something like that.
Thanks.
Cascardo.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 12/15] netdev-vport: Add IPv6 support for build/push/pop tunnel header
On Tue, Nov 10, 2015 at 04:05:59PM -0800, Ben Pfaff wrote: > On Thu, Oct 22, 2015 at 03:29:05PM -0200, Thadeu Lima de Souza Cascardo wrote: > > This includes VXLAN, GRE and Geneve. > > > > Signed-off-by: Thadeu Lima de Souza Cascardo > > If it's OK with you, I'll leave the review of this and the remaining > patches until the next round. Sure. I think this patch needs some special review, because of the datatypes used, some of the styles, protocols and other details. Cascardo. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] ovs-router: Report ovs/route/add errors as errors.
On Tue, Nov 10, 2015 at 04:13:47PM -0800, Ben Pfaff wrote:
> The _error version should be used to report errors.
>
> Also, add missing return in one error case.
>
> Signed-off-by: Ben Pfaff
> ---
> v1->v2: Add missing return in error case (thanks Cascardo!).
>
> lib/ovs-router.c | 12 +++-
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/lib/ovs-router.c b/lib/ovs-router.c
> index 2f093e8..619aa3a 100644
> --- a/lib/ovs-router.c
> +++ b/lib/ovs-router.c
> @@ -275,7 +275,8 @@ ovs_router_add(struct unixctl_conn *conn, int argc,
> gw6 = in6addr_any;
> }
> } else {
> -unixctl_command_reply(conn, "Invalid parameters");
> +unixctl_command_reply_error(conn, "Invalid parameters");
> +return;
This is OK, included in the first patch.
> }
> ovs_router_insert__(plen + 32, &ip6, plen, argv[2], &gw6);
> unixctl_command_reply(conn, "OK");
OK.
> @@ -293,13 +294,13 @@ ovs_router_del(struct unixctl_conn *conn, int argc
> OVS_UNUSED,
> in6_addr_set_mapped_ipv4(&ip6, ip);
> plen += 96;
> } else if (!scan_ipv6_route(argv[1], &ip6, &plen)) {
> -unixctl_command_reply(conn, "Invalid parameters");
> +unixctl_command_reply_error(conn, "Invalid parameters");
> }
My bad. This should have that return too. That amounts to three missing returns.
I guess there is a pattern here where we assume this implies a return. I am
covering the bases now, going though all changes.
Cascardo.
> if (rt_entry_delete(plen + 32, &ip6, plen)) {
> unixctl_command_reply(conn, "OK");
OK.
> seq_change(tnl_conf_seq);
> } else {
> -unixctl_command_reply(conn, "Not found");
> +unixctl_command_reply_error(conn, "Not found");
> }
> }
>
This is OK too. But should we do a return in these cases as well, establishing a
pattern?
The one in ovs_router_show is fine too.
> @@ -347,7 +348,8 @@ ovs_router_lookup_cmd(struct unixctl_conn *conn, int argc
> OVS_UNUSED,
> if (scan_ipv4_route(argv[1], &ip, &plen) && plen == 32) {
> in6_addr_set_mapped_ipv4(&ip6, ip);
> } else if (!(scan_ipv6_route(argv[1], &ip6, &plen) && plen == 128)) {
> -unixctl_command_reply(conn, "Invalid parameters");
> +unixctl_command_reply_error(conn, "Invalid parameters");
> +return;
> }
>
OK.
> if (ovs_router_lookup(&ip6, iface, &gw)) {
> @@ -358,7 +360,7 @@ ovs_router_lookup_cmd(struct unixctl_conn *conn, int argc
> OVS_UNUSED,
> unixctl_command_reply(conn, ds_cstr(&ds));
OK.
> ds_destroy(&ds);
> } else {
> -unixctl_command_reply(conn, "Not found");
> +unixctl_command_reply_error(conn, "Not found");
> }
> }
>
This doesn't need the return, but see above about a pattern.
> --
> 2.1.3
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] ovs-router: Report ovs/route/add errors as errors.
On Wed, Nov 11, 2015 at 12:19:25PM -0200, Thadeu Lima de Souza Cascardo wrote:
> On Tue, Nov 10, 2015 at 04:13:47PM -0800, Ben Pfaff wrote:
> > The _error version should be used to report errors.
> >
> > Also, add missing return in one error case.
> >
> > Signed-off-by: Ben Pfaff
> > ---
> > v1->v2: Add missing return in error case (thanks Cascardo!).
> >
> > lib/ovs-router.c | 12 +++-
> > 1 file changed, 7 insertions(+), 5 deletions(-)
> >
All other uses are fine, when considering only the context of the function they
are called in. Many of them use the pattern of using return or goto out/exit in
the same block as calling unixctl_command_reply{,_error}. No use of return when
at the end of the function in a if/else if/else block.
Cascardo.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 11/15] ofproto-dpif-xlate: Add IPv6 ND support for XC_TNL_ARP
On Thu, Nov 12, 2015 at 02:38:48AM -0200, Flavio Leitner wrote:
> On Wed, Nov 11, 2015 at 08:29:32AM -0800, Ben Pfaff wrote:
> > On Wed, Nov 11, 2015 at 11:57:04AM -0200, Thadeu Lima de Souza Cascardo
> > wrote:
> > > On Tue, Nov 10, 2015 at 04:03:13PM -0800, Ben Pfaff wrote:
> > > > On Thu, Oct 22, 2015 at 03:29:04PM -0200, Thadeu Lima de Souza Cascardo
> > > > wrote:
> > > > > Use IPv4-mapped addresses and use either tnl_arp_lookup or
> > > > > tnl_nd_lookup.
> > > > >
> > > > > Signed-off-by: Thadeu Lima de Souza Cascardo
> > > >
> > > > ...
> > > >
> > > > > @@ -5348,8 +5349,13 @@ xlate_push_stats(struct xlate_cache *xcache,
> > > > > break;
> > > > > case XC_TNL_ARP:
> > > > > /* Lookup arp to avoid arp timeout. */
> > > > > -tnl_arp_lookup(entry->u.tnl_arp_cache.br_name,
> > > > > - entry->u.tnl_arp_cache.d_ip, &dmac);
> > > > > +d_ip =
> > > > > in6_addr_get_mapped_ipv4(&entry->u.tnl_arp_cache.d_ipv6);
> > > > > +if (d_ip) {
> > > > > +tnl_arp_lookup(entry->u.tnl_arp_cache.br_name, d_ip,
> > > > > &dmac);
> > > > > +} else {
> > > > > +tnl_nd_lookup(entry->u.tnl_arp_cache.br_name,
> > > > > + &entry->u.tnl_arp_cache.d_ipv6,
> > > > > &dmac);
> > > > > +}
> > > >
> > > > This code seems a little silly to me, because it is going to some
> > > > trouble to distinguish IPv4 from IPv6 and pick the correct
> > > > tnl_*_lookup() function, and either function it picks is going to
> > > > convert that right back to do the lookup. I think it would be more
> > > > sensible to export tnl_arp_lookup__() so that the double conversion
> > > > isn't needed.
> > > >
> > > > Thanks,
> > > >
> > > > Ben.
> > >
> > > Urgh. Well, I could just use tnl_nd_lookup here with an extra comment
> > > about that. Exporting tnl_arp_lookup__ would require exporting struct
> > > tnl_arp_entry. But I see your point. The only bad point for me in
> > > using tnl_nd_lookup directly would be the naming. But since we already
> > > have an IPv6 address in our hands, why not just add a comment assuring
> > > this will work on IPv4-mapped addresses as well?
> >
> > If the naming is an issue, you could invent a more generic name,
> > e.g. "tnl_lookup_mac_to_ip_binding". But I don't think it's a big deal.
>
> maybe tnl_neigh_lookup() ?
>
> fbl
>
I thought of that, but since we are giving an IPv6 address to tnl_nd_lookup, we
could claim that tnl_neigh_lookup works with both IPv4 and IPv6 addresses. But I
would implement that using IPv4-mapped addresses anyway. And tnl_nd_lookup
already supports that. If there were two different databases, that could make
sense, but we have only one for both mappings. I think commenting that is good
enough.
Cascardo.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OVS bridges always report UP + hotplugging support?
On Fri, Nov 13, 2015 at 09:56:50AM -0800, Karol Mroz wrote: > Hi Thadeu, > > On Thu, Nov 5, 2015 at 4:51 AM, Thadeu Lima de Souza Cascardo < > casca...@redhat.com> wrote: > > > On Wed, Nov 04, 2015 at 02:06:56PM -0800, Karol Mroz wrote: > > > Hello, > > > > > > Support for configuring OVS bridges has been added to our (SUSE) network > > > manager. > > > During test (OVS 2.1.2, kernel 3.12.x), a couple things have been > > uncovered > > > that > > > I'd love to get some feedback on. > > > > > > 1. OVS bridges appear to report up state, regardless if this is true. For > > > example, > > > up is reported even if the underlying bridge port is down, or internally > > > OVS has > > > not yet set up the necessary flows to allow for data transmission. Is > > this > > > something > > > that is planned to be addressed? I realize 2.1.2 is an older version, so > > > perhaps > > > this has already been addressed in later versions of OVS? > > > > > > 2. OVS does not appear to handle hotplugging. For example, if a port, > > which > > > is attached > > > to the an OVS bridge (thus is correctly reflected in ovsdb) arrives on > > the > > > system _after_ > > > the actual `add-port` operation was performed, it will not get enslaved > > > under the ovs-system > > > interface. In this case, a manual `del-port/add-port` is needed. Is there > > > any plan to > > > add netlink monitoring of interface states (NEWLINK/DELLINK, etc) + > > > enslavement > > > when appropriate? > > > > > > > This has been implemented. Check commit > > e21c6643a02c6b446d2fbdfde366ea303b4c2730, which is not present in any > > release > > yet. > > > > Yes, I see. Thank you. This commit appears to rely on some additional > infrastructure being > in place (ie. various rtnetlink_*() APIs). Do you have some idea what > upcoming release would > contain this feature. Well, if you are on Linux, there is no problem. In fact, this was even implemented and tested on FreeBSD. Unless you have a kernel configured without rtnetlink, it will work. And if you don't have rtnetlink, other thinks wouldn't work on OVS as well. So, I don't understand what's your concern about it relying on that API on OVS. This feature should appear in the next release of OVS, whatever it's numbered, 2.5.0, or 3.0. > > Further, would this be useful in addressing point #1 above? > I don't know too much about point #1, but this feature does not address that, sorry. Do you mean that OVS reports the UP state to a controller using OpenFlow? Cascardo. > Thanks, > Karol ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OVS bridges always report UP + hotplugging support?
On Fri, Nov 13, 2015 at 04:30:12PM -0200, Thadeu Lima de Souza Cascardo wrote: > On Fri, Nov 13, 2015 at 09:56:50AM -0800, Karol Mroz wrote: > > Hi Thadeu, > > > > On Thu, Nov 5, 2015 at 4:51 AM, Thadeu Lima de Souza Cascardo < > > casca...@redhat.com> wrote: > > > > > On Wed, Nov 04, 2015 at 02:06:56PM -0800, Karol Mroz wrote: > > > > Hello, > > > > > > > > Support for configuring OVS bridges has been added to our (SUSE) network > > > > manager. > > > > During test (OVS 2.1.2, kernel 3.12.x), a couple things have been > > > uncovered > > > > that > > > > I'd love to get some feedback on. > > > > > > > > 1. OVS bridges appear to report up state, regardless if this is true. > > > > For > > > > example, > > > > up is reported even if the underlying bridge port is down, or internally > > > > OVS has > > > > not yet set up the necessary flows to allow for data transmission. Is > > > this > > > > something > > > > that is planned to be addressed? I realize 2.1.2 is an older version, so > > > > perhaps > > > > this has already been addressed in later versions of OVS? > > > > > > > > 2. OVS does not appear to handle hotplugging. For example, if a port, > > > which > > > > is attached > > > > to the an OVS bridge (thus is correctly reflected in ovsdb) arrives on > > > the > > > > system _after_ > > > > the actual `add-port` operation was performed, it will not get enslaved > > > > under the ovs-system > > > > interface. In this case, a manual `del-port/add-port` is needed. Is > > > > there > > > > any plan to > > > > add netlink monitoring of interface states (NEWLINK/DELLINK, etc) + > > > > enslavement > > > > when appropriate? > > > > > > > > > > This has been implemented. Check commit > > > e21c6643a02c6b446d2fbdfde366ea303b4c2730, which is not present in any > > > release > > > yet. > > > > > > > Yes, I see. Thank you. This commit appears to rely on some additional > > infrastructure being > > in place (ie. various rtnetlink_*() APIs). Do you have some idea what > > upcoming release would > > contain this feature. > > Well, if you are on Linux, there is no problem. In fact, this was even > implemented and tested on FreeBSD. Unless you have a kernel configured without > rtnetlink, it will work. And if you don't have rtnetlink, other thinks > wouldn't > work on OVS as well. So, I don't understand what's your concern about it > relying > on that API on OVS. I missed the fact that you may consider backport the feature. In that case, using new API may be a problem. But I believe it would be easy to backport it to the 2.4 series. 2.1, on the other hand, I guess it may be harder. Cascardo. > > This feature should appear in the next release of OVS, whatever it's numbered, > 2.5.0, or 3.0. > > > > > Further, would this be useful in addressing point #1 above? > > > > I don't know too much about point #1, but this feature does not address that, > sorry. > > Do you mean that OVS reports the UP state to a controller using OpenFlow? > > Cascardo. > > > Thanks, > > Karol > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] prevent test failures when there are non Ethernet devices on the system
When there are PtP TUN devices on the system or SIT devices, tests will fail
because of a warning that it was not possible to get their Ethernet addresses.
That call comes from the route code adding tunnel ports.
Make that warning an informational message and filter that out during tests.
Also, return EINVAL when trying to get those interface Ethernet addresses, which
will prevent them from being added to the tunnel ports pool and will properly
fail in other places as well.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/netdev-linux.c | 3 ++-
tests/ofproto-macros.at | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 584e804..e047be5 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -5516,8 +5516,9 @@ get_etheraddr(const char *netdev_name, struct eth_addr
*ea)
}
hwaddr_family = ifr.ifr_hwaddr.sa_family;
if (hwaddr_family != AF_UNSPEC && hwaddr_family != ARPHRD_ETHER) {
-VLOG_WARN("%s device has unknown hardware address family %d",
+VLOG_INFO("%s device has unknown hardware address family %d",
netdev_name, hwaddr_family);
+return EINVAL;
}
memcpy(ea, ifr.ifr_hwaddr.sa_data, ETH_ADDR_LEN);
return 0;
diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
index fcd36fb..e808258 100644
--- a/tests/ofproto-macros.at
+++ b/tests/ofproto-macros.at
@@ -268,6 +268,7 @@ m4_define([_OVS_VSWITCHD_START],
/vswitchd|INFO|ovs-vswitchd (Open vSwitch)/d
/reconnect|INFO|/d
/ofproto|INFO|using datapath ID/d
+/netdev_linux|INFO|.*device has unknown hardware address family/d
/ofproto|INFO|datapath ID changed to fedcba9876543210/d']])
])
--
2.5.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3] ovs-router: Report ovs/route/add errors as errors.
On Mon, Nov 23, 2015 at 08:49:35PM -0800, Ben Pfaff wrote: > The _error version should be used to report errors. > > Also, add missing return in one error case. > > Signed-off-by: Ben Pfaff > --- > v1->v2: Add missing return in error case (thanks Cascardo!). > v2->v3: Add missing return in another error case (thanks Cascardo!). > Acked-by: Thadeu Lima de Souza Cascardo ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] datapath/vport: Remove an invalid comment
On Tue, Nov 24, 2015 at 03:28:35PM -0500, Aaron Conole wrote:
> Commit 3544358aa596 ("datapath: Improve kernel hash table") removed the
> failure condition of ovs_vport_del by switching away from a custom
> hashtable which allocated a new bucket, to an hlist with flexible-array
> buckets. The function stopped returning failure code at that point, so
> remove the misleading comment.
>
> Signed-off-by: Aaron Conole
> ---
> datapath/vport.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/datapath/vport.c b/datapath/vport.c
> index 024491f..e000e8c 100644
> --- a/datapath/vport.c
> +++ b/datapath/vport.c
> @@ -266,8 +266,8 @@ int ovs_vport_set_options(struct vport *vport, struct
> nlattr *options)
> *
> * @vport: vport to delete.
> *
> - * Detaches @vport from its datapath and destroys it. It is possible to fail
> - * for reasons such as lack of memory. ovs_mutex must be held.
> + * Detaches @vport from its datapath and destroys it. ovs_mutex must be
> + * held.
> */
> void ovs_vport_del(struct vport *vport)
> {
> --
> 2.6.1.133.gf5b6079
Acked-by: Thadeu Lima de Souza Cascardo
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2 00/14] IPv6 tunneling in user space
This is a new patchset for IPv6 tunneling in user space. IPv4-mapped addresses are used in most places, except in flow_tnl because that proved to be a challenge to do right because of the automated handling of MFF_TUN_DST and MFF_TUN_SRC. For v2, I changed smap_add_ipv6, renamed tnl-arp-cache to tnl-neigh-cache, added compose_nd to lib/packets.c, and fixed some style and casting. Jiri Benc (3): tunneling: add IPv6 support to netdev_tunnel_config lib: add format_in6_addr and scan_in6_addr tunneling: extend flow_tnl with ipv6 addresses Thadeu Lima de Souza Cascardo (11): tnl-ports: remove unused tnl-arp-cache.h and include other necessary headers rename tnl-arp-cache module and functions to tnl-neigh-cache Add ipv6_string_mapped. smap: smap_add_ipv6 adds IPv4-mapped addresses as IPv4 ofproto-dpif-xlate: use IPv6 for orig_tunnel_ip_dst ofproto-dpif-xlate: use IPv6 for tnl_neigh_cache netdev-vport: Add IPv6 support for build/push/pop tunnel header ofproto-dpif-xlate: Support IPv6 when sending to tunnel tnl-neigh-cache: remove tnl_arp_lookup Allow flow-based IPv6 tunnels to be configured with OpenFlow tests: Add tunnel-push-pop-ipv6 tests datapath/linux/compat/include/linux/openvswitch.h | 2 + lib/automake.mk | 4 +- lib/dpif-netdev.c | 4 +- lib/dpif.c| 10 +- lib/flow.c| 30 +- lib/flow.h| 4 +- lib/match.c | 36 ++- lib/match.h | 6 + lib/meta-flow.c | 42 +++ lib/meta-flow.h | 35 +++ lib/netdev-dummy.c| 58 +++- lib/netdev-vport.c| 319 +++--- lib/netdev.h | 4 +- lib/nx-match.c| 6 +- lib/odp-util.c| 165 --- lib/odp-util.h| 8 +- lib/ofp-util.c| 2 +- lib/packets.c | 90 ++ lib/packets.h | 47 +++- lib/smap.c| 4 +- lib/tnl-arp-cache.c | 316 - lib/tnl-arp-cache.h | 45 --- lib/tnl-neigh-cache.c | 312 + lib/tnl-neigh-cache.h | 41 +++ lib/tnl-ports.c | 3 +- ofproto/ofproto-dpif-rid.c| 3 +- ofproto/ofproto-dpif-rid.h| 4 +- ofproto/ofproto-dpif-xlate.c | 124 ++--- ofproto/ofproto-tnl-unixctl.man | 3 + ofproto/tunnel.c | 76 -- ofproto/tunnel.h | 5 +- tests/automake.mk | 1 + tests/ofproto-macros.at | 2 +- tests/ofproto.at | 4 +- tests/testsuite.at| 1 + tests/tunnel-push-pop-ipv6.at | 153 +++ tests/tunnel-push-pop.at | 2 +- 37 files changed, 1358 insertions(+), 613 deletions(-) delete mode 100644 lib/tnl-arp-cache.c delete mode 100644 lib/tnl-arp-cache.h create mode 100644 lib/tnl-neigh-cache.c create mode 100644 lib/tnl-neigh-cache.h create mode 100644 tests/tunnel-push-pop-ipv6.at -- 2.5.0 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2 04/14] smap: smap_add_ipv6 adds IPv4-mapped addresses as IPv4
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/smap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/smap.c b/lib/smap.c
index 2c41013..07dd23a 100644
--- a/lib/smap.c
+++ b/lib/smap.c
@@ -19,6 +19,7 @@
#include "hash.h"
#include "json.h"
+#include "packets.h"
#include "uuid.h"
static struct smap_node *smap_add__(struct smap *, char *, void *,
@@ -101,8 +102,7 @@ void
smap_add_ipv6(struct smap *smap, const char *key, struct in6_addr *addr)
{
char buf[INET6_ADDRSTRLEN];
-
-inet_ntop(AF_INET6, addr, buf, sizeof buf);
+ipv6_string_mapped(buf, addr);
smap_add(smap, key, buf);
}
--
2.5.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2 03/14] Add ipv6_string_mapped.
ipv6_string_mapped stores an IPv6 or IPv4 representation of an IPv6 address into
a string. If the address is IPv4-mapped, it's represented in IPv4 dotted-decimal
format.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/packets.c | 15 +++
lib/packets.h | 1 +
2 files changed, 16 insertions(+)
diff --git a/lib/packets.c b/lib/packets.c
index f6fd480..c790087 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -474,6 +474,21 @@ ipv6_format_masked(const struct in6_addr *addr, const
struct in6_addr *mask,
}
}
+/* Stores the string representation of the IPv6 address 'addr' into the
+ * character array 'addr_str', which must be at least INET6_ADDRSTRLEN
+ * bytes long. If addr is IPv4-mapped, store an IPv4 dotted-decimal string. */
+const char *
+ipv6_string_mapped(char *addr_str, const struct in6_addr *addr)
+{
+ovs_be32 ip;
+ip = in6_addr_get_mapped_ipv4(addr);
+if (ip) {
+return inet_ntop(AF_INET, &ip, addr_str, INET6_ADDRSTRLEN);
+} else {
+return inet_ntop(AF_INET6, addr, addr_str, INET6_ADDRSTRLEN);
+}
+}
+
struct in6_addr ipv6_addr_bitand(const struct in6_addr *a,
const struct in6_addr *b)
{
diff --git a/lib/packets.h b/lib/packets.h
index 188cf84..d5277ab 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -954,6 +954,7 @@ void ipv6_format_addr(const struct in6_addr *addr, struct
ds *);
void ipv6_format_mapped(const struct in6_addr *addr, struct ds *);
void ipv6_format_masked(const struct in6_addr *addr,
const struct in6_addr *mask, struct ds *);
+const char * ipv6_string_mapped(char *addr_str, const struct in6_addr *addr);
struct in6_addr ipv6_addr_bitand(const struct in6_addr *src,
const struct in6_addr *mask);
struct in6_addr ipv6_create_mask(int mask);
--
2.5.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
