[ovs-dev] ovn: support ARP response for known IPs

For lswitch ports with known IPs, ARP is responded directly from
local ovn-controller to avoid flooding.

Signed-off-by: Han Zhou <zhouhan@gmail.com>
---
 ovn/northd/ovn-northd.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

      * (priority 100). */
     HMAP_FOR_EACH (op, key_node, ports) {

On 11/15/2015 02:11 AM, Han Zhou wrote:
> For lswitch ports with known IPs, ARP is responded directly from
> local ovn-controller to avoid flooding.
>
> Signed-off-by: Han Zhou <zhouhan@gmail.com>
> ---
>  ovn/northd/ovn-northd.c | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 8fe0c2c..c072224 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -1151,6 +1151,45 @@ build_lswitch_flows(struct hmap *datapaths, struct
> hmap *ports,
Looks like there is a word wrap here, so I couldn't apply the patch using git-am.
>          ds_destroy(&match);
>      }
>
> +    /* Ingress table 3: Destination lookup, ARP reply for known IPs.
> +     * (priority 150). */
> +    HMAP_FOR_EACH (op, key_node, ports) {
> +        if (!op->nbs) {
> +            continue;
> +        }
> +
> +        for (size_t i = 0; i < op->nbs->n_addresses; i++) {
> +            struct eth_addr ea;
> +            ovs_be32 ip;
> +
> +            if (ovs_scan(op->nbs->addresses[i],
> +                         ETH_ADDR_SCAN_FMT" "IP_SCAN_FMT,
> +                         ETH_ADDR_SCAN_ARGS(ea), IP_SCAN_ARGS(&ip))) {
> +                char *match = xasprintf(
> +                    "arp.tpa == "IP_FMT" && arp.op == 1", IP_ARGS(ip));
> +                char *actions = xasprintf(
> +                    "eth.dst = eth.src; "
> +                    "eth.src = "ETH_ADDR_FMT"; "
> +                    "arp.op = 2; /* ARP reply */ "
> +                    "arp.tha = arp.sha; "
> +                    "arp.sha = "ETH_ADDR_FMT"; "
> +                    "arp.tpa = arp.spa; "
> +                    "arp.spa = "IP_FMT"; "
> +                    "outport = inport; "
> +                    "inport = \"\"; /* Allow sending out inport. */ "
> +                    "output;",
> +                    ETH_ADDR_ARGS(ea),
> +                    ETH_ADDR_ARGS(ea),
> +                    IP_ARGS(ip),
> +                    op->json_key);
op->json(key) is not required as there are only 3 formatters to xasprintf.

I tested this patch and it is working as expected.
Just one comment though - When I create a logical port which is still not bound to any vif (ie. just do "neutron port-create")
and try to do arping to the ip address of this port, I get an arp reply, which ideally should not happen.
Not sure if it is a big concern though.


> +                ovn_lflow_add(lflows, op->od, S_SWITCH_IN_L2_LKUP, 150,
> +                              match, actions);
> +                free(match);
> +                free(actions);
> +            }
> +        }
> +    }
> +
>      /* Ingress table 3: Destination lookup, broadcast and multicast
> handling
>       * (priority 100). */
>      HMAP_FOR_EACH (op, key_node, ports) {

Thanks
Numan

On Thu, Nov 19, 2015 at 8:10 AM, Numan Siddique <nusiddiq@redhat.com> wrote:

> On 11/15/2015 02:11 AM, Han Zhou wrote:
> > For lswitch ports with known IPs, ARP is responded directly from
> > local ovn-controller to avoid flooding.
> >
> > Signed-off-by: Han Zhou <zhouhan@gmail.com>
>

I'm curious what Ben has to say about this.  I know he was doing some ARP
related work for OVN, but I haven't seen it yet.  Maybe they don't overlap.


> > ---
> >  ovn/northd/ovn-northd.c | 39 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 39 insertions(+)
> >
> > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> > index 8fe0c2c..c072224 100644
> > --- a/ovn/northd/ovn-northd.c
> > +++ b/ovn/northd/ovn-northd.c
> > @@ -1151,6 +1151,45 @@ build_lswitch_flows(struct hmap *datapaths, struct
> > hmap *ports,
> Looks like there is a word wrap here, so I couldn't apply the patch using
> git-am.
>

Using "git send-email" helps avoid these types of problems.


> >          ds_destroy(&match);
> >      }
> >
> > +    /* Ingress table 3: Destination lookup, ARP reply for known IPs.
> > +     * (priority 150). */
> > +    HMAP_FOR_EACH (op, key_node, ports) {
> > +        if (!op->nbs) {
> > +            continue;
> > +        }
> > +
> > +        for (size_t i = 0; i < op->nbs->n_addresses; i++) {
> > +            struct eth_addr ea;
> > +            ovs_be32 ip;
> > +
> > +            if (ovs_scan(op->nbs->addresses[i],
> > +                         ETH_ADDR_SCAN_FMT" "IP_SCAN_FMT,
> > +                         ETH_ADDR_SCAN_ARGS(ea), IP_SCAN_ARGS(&ip))) {
> > +                char *match = xasprintf(
> > +                    "arp.tpa == "IP_FMT" && arp.op == 1", IP_ARGS(ip));
> > +                char *actions = xasprintf(
> > +                    "eth.dst = eth.src; "
> > +                    "eth.src = "ETH_ADDR_FMT"; "
> > +                    "arp.op = 2; /* ARP reply */ "
> > +                    "arp.tha = arp.sha; "
> > +                    "arp.sha = "ETH_ADDR_FMT"; "
> > +                    "arp.tpa = arp.spa; "
> > +                    "arp.spa = "IP_FMT"; "
> > +                    "outport = inport; "
> > +                    "inport = \"\"; /* Allow sending out inport. */ "
> > +                    "output;",
> > +                    ETH_ADDR_ARGS(ea),
> > +                    ETH_ADDR_ARGS(ea),
> > +                    IP_ARGS(ip),
> > +                    op->json_key);
> op->json(key) is not required as there are only 3 formatters to xasprintf.
>
> I tested this patch and it is working as expected.
> Just one comment though - When I create a logical port which is still not
> bound to any vif (ie. just do "neutron port-create")
> and try to do arping to the ip address of this port, I get an arp reply,
> which ideally should not happen.
> Not sure if it is a big concern though.
>

Good point.  I think it's fine though.


>
> > +                ovn_lflow_add(lflows, op->od, S_SWITCH_IN_L2_LKUP, 150,
> > +                              match, actions);
> > +                free(match);
> > +                free(actions);
> > +            }
> > +        }
> > +    }
> > +
> >      /* Ingress table 3: Destination lookup, broadcast and multicast
> > handling
> >       * (priority 100). */
> >      HMAP_FOR_EACH (op, key_node, ports) {
>
> Thanks
> Numan
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>