Re: [ovs-dev] [PATCH] ovn: Do not reply to ARP or ND NS for a VM's own IP address.

2016-09-30 Thread Ben Pfaff
On Fri, Sep 30, 2016 at 04:45:22PM +0500, Valentine Sinitsyn wrote:
> Hi Ben,
> 
> On 30.09.2016 02:55, Ben Pfaff wrote:
> >On Thu, Sep 29, 2016 at 12:15:36PM -0700, Han Zhou wrote:
> >>On Thu, Sep 29, 2016 at 11:31 AM, Ben Pfaff  wrote:
> >>>
> >>>When a VM sends an ARP or an ND NS for its own IP address, it is trying to
> >>>check for a duplicate address in the network.  OVN needs to suppress the
> >>>reply in such a case, otherwise the VM thinks that its address is a
> >>>duplicate.
> >>>
> >>>Reported-by: Valentine Sinitsyn 
> >>>Reported-at:
> >>http://openvswitch.org/pipermail/dev/2016-September/080037.html
> >>>Signed-off-by: Ben Pfaff 
> >>
> >>+1
> >>
> >>It will also solve a problem when there are nested VMs (or network
> >>namespaces) within the source VM, connected to VM port by a bridge inside,
> >>sending ARP for the VM's IP, they would get duplicated responses.
> >
> >Thanks for the review.
> >
> >I'm hoping to hear from Valentine whether this fixes the originally
> >reported problem in practice.
> First of all, thanks for the patch. In fact, we tried a pretty similar
> approach yesterday, and it seemed to help. Your patch solves the problem as
> well.
> 
> Tested-by: Valentine Sinitsyn 

Thanks for testing!

I applied this to master and branch-2.6.
___
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ovn: Do not reply to ARP or ND NS for a VM's own IP address.

2016-09-30 Thread Valentine Sinitsyn

Hi Ben,

On 30.09.2016 02:55, Ben Pfaff wrote:

On Thu, Sep 29, 2016 at 12:15:36PM -0700, Han Zhou wrote:

On Thu, Sep 29, 2016 at 11:31 AM, Ben Pfaff  wrote:


When a VM sends an ARP or an ND NS for its own IP address, it is trying to
check for a duplicate address in the network.  OVN needs to suppress the
reply in such a case, otherwise the VM thinks that its address is a
duplicate.

Reported-by: Valentine Sinitsyn 
Reported-at:

http://openvswitch.org/pipermail/dev/2016-September/080037.html

Signed-off-by: Ben Pfaff 


+1

It will also solve a problem when there are nested VMs (or network
namespaces) within the source VM, connected to VM port by a bridge inside,
sending ARP for the VM's IP, they would get duplicated responses.


Thanks for the review.

I'm hoping to hear from Valentine whether this fixes the originally
reported problem in practice.
First of all, thanks for the patch. In fact, we tried a pretty similar 
approach yesterday, and it seemed to help. Your patch solves the problem 
as well.


Tested-by: Valentine Sinitsyn 

Best,
Valentine
___
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ovn: Do not reply to ARP or ND NS for a VM's own IP address.

2016-09-29 Thread Ben Pfaff
On Thu, Sep 29, 2016 at 03:13:47PM -0700, Darrell Ball wrote:
> On Thu, Sep 29, 2016 at 11:31 AM, Ben Pfaff  wrote:
> 
> > When a VM sends an ARP or an ND NS for its own IP address, it is trying to
> > check for a duplicate address in the network.  OVN needs to suppress the
> > reply in such a case, otherwise the VM thinks that its address is a
> > duplicate.
> >
> > Reported-by: Valentine Sinitsyn 
> > Reported-at: http://openvswitch.org/pipermail/dev/2016-September/
> > 080037.html
> > Signed-off-by: Ben Pfaff 

...

> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index 677ab46..68885b9 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -1159,13 +1159,18 @@ for is in 1 2 3; do
> >  sip=`ip_to_hex 192 168 0 $i$j`
> >  tip=`ip_to_hex 192 168 0 $id$jd`
> >  tip_unknown=`ip_to_hex 11 11 11 11`
> > -test_arp $s f0$s $sip $tip f0$d
> >   #9
> > +if test $d != $s; then
> > +reply_ha=f0$d
> > +else
> > +reply_ha=
> >
> 
> Would it be helpful to update bullet "9" in the preceding description?
> 
> 
> 
> > +fi
> > +test_arp $s f0$s $sip $tip $reply_ha
> >  #9
> >  test_arp $s f0$s $sip $tip_unknown
> >  #10
> >
> >  if test $jd = 3; then
> >  # lsp[123]3 has an additional ip 192.169.0.[123]3.
> >  tip=`ip_to_hex 192 169 0 $id$jd`
> > -test_arp $s f0$s $sip $tip f0$d
> >   #9
> > +test_arp $s f0$s $sip $tip $reply_ha
> >  #9
> >  fi
> >  done
> >  done
> > @@ -1413,13 +1418,14 @@ for s in 1 2 3; do
> >  sip=192.168.0.$s
> >  tip=192.168.0.$d
> >  tip_unknown=11.11.11.11
> > -test_arp $s f0:00:00:00:00:0$s $sip $tip f0:00:00:00:00:0$d
> >   #9
> > +if test $d != $s; then reply_ha=f0:00:00:00:00:0$d; else
> > reply_ha=; fi
> >
> 
> Would it be helpful to update bullet "9." in the preceding description?
> 
> 
> > +test_arp $s f0:00:00:00:00:0$s $sip $tip $reply_ha
> >  #9
> >  test_arp $s f0:00:00:00:00:0$s $sip $tip_unknown
> >  #10
> >
> >  if test $d = 3; then
> >  # lp3 has an additional ip 192.169.0.[123]3.
> >  tip=192.169.0.$d
> > -test_arp $s f0:00:00:00:00:0$s $sip $tip f0:00:00:00:00:0$d
> >   #9
> > +test_arp $s f0:00:00:00:00:0$s $sip $tip $reply_ha
> >  #9
> >  fi
> >  done
> >
> > --
> > 2.1.3
> >
> >
> Acked by: Darrell Ball 

Thanks for the review and the suggestions.  I'll update the comments.
___
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ovn: Do not reply to ARP or ND NS for a VM's own IP address.

2016-09-29 Thread Darrell Ball
On Thu, Sep 29, 2016 at 11:31 AM, Ben Pfaff  wrote:

> When a VM sends an ARP or an ND NS for its own IP address, it is trying to
> check for a duplicate address in the network.  OVN needs to suppress the
> reply in such a case, otherwise the VM thinks that its address is a
> duplicate.
>
> Reported-by: Valentine Sinitsyn 
> Reported-at: http://openvswitch.org/pipermail/dev/2016-September/
> 080037.html
> Signed-off-by: Ben Pfaff 
> ---
>  ovn/northd/ovn-northd.8.xml | 23 +++
>  ovn/northd/ovn-northd.c | 22 ++
>  tests/ovn.at| 14 ++
>  3 files changed, 55 insertions(+), 4 deletions(-)
>
> diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
> index 1da633a..77eb3d1 100644
> --- a/ovn/northd/ovn-northd.8.xml
> +++ b/ovn/northd/ovn-northd.8.xml
> @@ -479,6 +479,29 @@ nd_na {
>
>
>
> +
> +  Priority-100 flows with match criteria like the ARP and ND flows
> +  above, except that they only match packets from the
> +  inport that owns the IP addresses in question, with
> +  action next;.  These flows prevent OVN from
> replying to,
> +  for example, an ARP request emitted by a VM for its own IP
> address.
> +  A VM only makes this kind of request to attempt to detect a
> duplicate
> +  IP address assignment, so sending a reply will prevent the VM
> from
> +  accepting the IP address that it owns.
> +
> +
> +
> +  In place of next;, it would be reasonable to use
> +  drop; for the flows' actions.  If everything is
> working
> +  as it is configured, then this would produce equivalent results,
> +  since no host should reply to the request.  But ARPing for
> one's own
> +  IP address is intended to detect situations where the network
> is not
> +  working as configured, so dropping the request would frustrate
> that
> +  intent.
> +
> +  
> +
> +  
>  One priority-0 fallback flow that matches all packets and
> advances to
>  the next table.
>
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index cdc5525..eeeb41d 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -2728,6 +2728,22 @@ build_lswitch_flows(struct hmap *datapaths, struct
> hmap *ports,
>  op->lsp_addrs[i].ipv4_addrs[j].addr_s);
>  ovn_lflow_add(lflows, op->od, S_SWITCH_IN_ARP_ND_RSP, 50,
>ds_cstr(&match), ds_cstr(&actions));
> +
> +/* Do not reply to an ARP request from the port that owns
> the
> + * address (otherwise a DHCP client that ARPs to check
> for a
> + * duplicate address will fail).  Instead, forward it the
> usual
> + * way.
> + *
> + * (Another alternative would be to simply drop the
> packet.  If
> + * everything is working as it is configured, then this
> would
> + * produce equivalent results, since no one should reply
> to the
> + * request.  But ARPing for one's own IP address is
> intended to
> + * detect situations where the network is not working as
> + * configured, so dropping the request would frustrate
> that
> + * intent.) */
> +ds_put_format(&match, " && inport == %s", op->json_key);
> +ovn_lflow_add(lflows, op->od, S_SWITCH_IN_ARP_ND_RSP, 100,
> +  ds_cstr(&match), "next;");
>  }
>
>  /* For ND solicitations, we need to listen for both the
> @@ -2758,6 +2774,12 @@ build_lswitch_flows(struct hmap *datapaths, struct
> hmap *ports,
>  op->lsp_addrs[i].ea_s);
>  ovn_lflow_add(lflows, op->od, S_SWITCH_IN_ARP_ND_RSP, 50,
>ds_cstr(&match), ds_cstr(&actions));
> +
> +/* Do not reply to a solicitation from the port that owns
> the
> + * address (otherwise DAD detection will fail). */
> +ds_put_format(&match, " && inport == %s", op->json_key);
> +ovn_lflow_add(lflows, op->od, S_SWITCH_IN_ARP_ND_RSP, 100,
> +  ds_cstr(&match), "next;");
>  }
>  }
>  }
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 677ab46..68885b9 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -1159,13 +1159,18 @@ for is in 1 2 3; do
>  sip=`ip_to_hex 192 168 0 $i$j`
>  tip=`ip_to_hex 192 168 0 $id$jd`
>  tip_unknown=`ip_to_hex 11 11 11 11`
> -test_arp $s f0$s $sip $tip f0$d
>   #9
> +if test $d != $s; then
> +reply_ha=f0$d
> +else
> +reply_ha=
>

Woul

Re: [ovs-dev] [PATCH] ovn: Do not reply to ARP or ND NS for a VM's own IP address.

2016-09-29 Thread Ben Pfaff
On Thu, Sep 29, 2016 at 12:15:36PM -0700, Han Zhou wrote:
> On Thu, Sep 29, 2016 at 11:31 AM, Ben Pfaff  wrote:
> >
> > When a VM sends an ARP or an ND NS for its own IP address, it is trying to
> > check for a duplicate address in the network.  OVN needs to suppress the
> > reply in such a case, otherwise the VM thinks that its address is a
> > duplicate.
> >
> > Reported-by: Valentine Sinitsyn 
> > Reported-at:
> http://openvswitch.org/pipermail/dev/2016-September/080037.html
> > Signed-off-by: Ben Pfaff 
>
> +1
> 
> It will also solve a problem when there are nested VMs (or network
> namespaces) within the source VM, connected to VM port by a bridge inside,
> sending ARP for the VM's IP, they would get duplicated responses.

Thanks for the review.

I'm hoping to hear from Valentine whether this fixes the originally
reported problem in practice.
___
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ovn: Do not reply to ARP or ND NS for a VM's own IP address.

2016-09-29 Thread Han Zhou
On Thu, Sep 29, 2016 at 11:31 AM, Ben Pfaff  wrote:
>
> When a VM sends an ARP or an ND NS for its own IP address, it is trying to
> check for a duplicate address in the network.  OVN needs to suppress the
> reply in such a case, otherwise the VM thinks that its address is a
> duplicate.
>
> Reported-by: Valentine Sinitsyn 
> Reported-at:
http://openvswitch.org/pipermail/dev/2016-September/080037.html
> Signed-off-by: Ben Pfaff 
> ---
>  ovn/northd/ovn-northd.8.xml | 23 +++
>  ovn/northd/ovn-northd.c | 22 ++
>  tests/ovn.at| 14 ++
>  3 files changed, 55 insertions(+), 4 deletions(-)
>
> diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
> index 1da633a..77eb3d1 100644
> --- a/ovn/northd/ovn-northd.8.xml
> +++ b/ovn/northd/ovn-northd.8.xml
> @@ -479,6 +479,29 @@ nd_na {
>
>
>
> +
> +  Priority-100 flows with match criteria like the ARP and ND
flows
> +  above, except that they only match packets from the
> +  inport that owns the IP addresses in question,
with
> +  action next;.  These flows prevent OVN from
replying to,
> +  for example, an ARP request emitted by a VM for its own IP
address.
> +  A VM only makes this kind of request to attempt to detect a
duplicate
> +  IP address assignment, so sending a reply will prevent the VM
from
> +  accepting the IP address that it owns.
> +
> +
> +
> +  In place of next;, it would be reasonable to use
> +  drop; for the flows' actions.  If everything is
working
> +  as it is configured, then this would produce equivalent
results,
> +  since no host should reply to the request.  But ARPing for
one's own
> +  IP address is intended to detect situations where the network
is not
> +  working as configured, so dropping the request would frustrate
that
> +  intent.
> +
> +  
> +
> +  
>  One priority-0 fallback flow that matches all packets and
advances to
>  the next table.
>
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index cdc5525..eeeb41d 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -2728,6 +2728,22 @@ build_lswitch_flows(struct hmap *datapaths, struct
hmap *ports,
>  op->lsp_addrs[i].ipv4_addrs[j].addr_s);
>  ovn_lflow_add(lflows, op->od, S_SWITCH_IN_ARP_ND_RSP, 50,
>ds_cstr(&match), ds_cstr(&actions));
> +
> +/* Do not reply to an ARP request from the port that
owns the
> + * address (otherwise a DHCP client that ARPs to check
for a
> + * duplicate address will fail).  Instead, forward it
the usual
> + * way.
> + *
> + * (Another alternative would be to simply drop the
packet.  If
> + * everything is working as it is configured, then this
would
> + * produce equivalent results, since no one should reply
to the
> + * request.  But ARPing for one's own IP address is
intended to
> + * detect situations where the network is not working as
> + * configured, so dropping the request would frustrate
that
> + * intent.) */
> +ds_put_format(&match, " && inport == %s", op->json_key);
> +ovn_lflow_add(lflows, op->od, S_SWITCH_IN_ARP_ND_RSP,
100,
> +  ds_cstr(&match), "next;");
>  }
>
>  /* For ND solicitations, we need to listen for both the
> @@ -2758,6 +2774,12 @@ build_lswitch_flows(struct hmap *datapaths, struct
hmap *ports,
>  op->lsp_addrs[i].ea_s);
>  ovn_lflow_add(lflows, op->od, S_SWITCH_IN_ARP_ND_RSP, 50,
>ds_cstr(&match), ds_cstr(&actions));
> +
> +/* Do not reply to a solicitation from the port that
owns the
> + * address (otherwise DAD detection will fail). */
> +ds_put_format(&match, " && inport == %s", op->json_key);
> +ovn_lflow_add(lflows, op->od, S_SWITCH_IN_ARP_ND_RSP,
100,
> +  ds_cstr(&match), "next;");
>  }
>  }
>  }
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 677ab46..68885b9 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -1159,13 +1159,18 @@ for is in 1 2 3; do
>  sip=`ip_to_hex 192 168 0 $i$j`
>  tip=`ip_to_hex 192 168 0 $id$jd`
>  tip_unknown=`ip_to_hex 11 11 11 11`
> -test_arp $s f0$s $sip $tip f0$d
   #9
> +if test $d != $s; then
> +reply_ha=f0$d
> +else
> +reply_ha=
> +fi
> +test_arp