With a few remarks below,

Acked-by: Jarno Rajahalme <[email protected]>

> On Jan 24, 2017, at 9:57 PM, Pravin B Shelar <[email protected]> wrote:
> 
> OVS router is basically partial copy of linux kernel FIB.
> kernel routing table uses skb-mark along with usual routing
> parameters. Following patch brings in support for skb-mark
> to ovs-router so that we can lookup route for given skb-mark.
> 
> Signed-off-by: Pravin B Shelar <[email protected]>
> ---
> v1-v2:
> Removed ovs/route2/add command
> reverted change to plen variable type.
> ---
> lib/netdev-vport.c           |   4 +-
> lib/ovs-router.c             | 125 ++++++++++++++++++++++++++++++-------------
> lib/ovs-router.h             |   6 ++-
> lib/route-table.c            |   2 +-
> ofproto/ofproto-dpif-sflow.c |   5 +-
> ofproto/ofproto-dpif-xlate.c |   2 +-
> tests/ovs-router.at          |  42 +++++++++++++++
> tests/tunnel-push-pop.at     |   3 ++
> 8 files changed, 147 insertions(+), 42 deletions(-)
> 
> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
> index 88b0bcf..2d0aa43 100644
> --- a/lib/netdev-vport.c
> +++ b/lib/netdev-vport.c
> @@ -260,10 +260,12 @@ tunnel_check_status_change__(struct netdev_vport 
> *netdev)
>     bool status = false;
>     struct in6_addr *route;
>     struct in6_addr gw;
> +    uint32_t mark;
> 
>     iface[0] = '\0';
>     route = &netdev->tnl_cfg.ipv6_dst;
> -    if (ovs_router_lookup(route, iface, NULL, &gw)) {
> +    mark = netdev->tnl_cfg.egress_pkt_mark;
> +    if (ovs_router_lookup(mark, route, iface, NULL, &gw)) {
>         struct netdev *egress_netdev;
> 
>         if (!netdev_open(iface, NULL, &egress_netdev)) {
> diff --git a/lib/ovs-router.c b/lib/ovs-router.c
> index 935b60a..d30eb3c 100644
> --- a/lib/ovs-router.c
> +++ b/lib/ovs-router.c
> @@ -45,6 +45,11 @@
> #include "unaligned.h"
> #include "unixctl.h"
> #include "util.h"
> +#include "openvswitch/vlog.h"
> +
> +VLOG_DEFINE_THIS_MODULE(ovs_router);
> +
> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> 
> static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER;
> static struct classifier cls;
> @@ -57,6 +62,7 @@ struct ovs_router_entry {
>     struct in6_addr src_addr;
>     uint8_t plen;
>     uint8_t priority;
> +    uint32_t mark;
> };
> 
> static struct ovs_router_entry *
> @@ -88,11 +94,12 @@ ovs_router_lookup_fallback(const struct in6_addr 
> *ip6_dst, char output_bridge[],
> }
> 
> bool
> -ovs_router_lookup(const struct in6_addr *ip6_dst, char output_bridge[],
> +ovs_router_lookup(uint32_t mark, const struct in6_addr *ip6_dst,
> +                  char output_bridge[],
>                   struct in6_addr *src, struct in6_addr *gw)
> {
>     const struct cls_rule *cr;
> -    struct flow flow = {.ipv6_dst = *ip6_dst};
> +    struct flow flow = {.ipv6_dst = *ip6_dst, .pkt_mark = mark};
> 
>     cr = classifier_lookup(&cls, OVS_VERSION_MAX, &flow, NULL);
>     if (cr) {
> @@ -115,7 +122,8 @@ rt_entry_free(struct ovs_router_entry *p)
>     free(p);
> }
> 
> -static void rt_init_match(struct match *match, const struct in6_addr 
> *ip6_dst,
> +static void rt_init_match(struct match *match, uint32_t mark,
> +                          const struct in6_addr *ip6_dst,
>                           uint8_t plen)
> {
>     struct in6_addr dst;
> @@ -127,6 +135,8 @@ static void rt_init_match(struct match *match, const 
> struct in6_addr *ip6_dst,
>     memset(match, 0, sizeof *match);
>     match->flow.ipv6_dst = dst;
>     match->wc.masks.ipv6_dst = mask;
> +    match->wc.masks.pkt_mark = UINT32_MAX;
> +    match->flow.pkt_mark = mark;
> }
> 
> static int
> @@ -178,7 +188,8 @@ out:
> }
> 
> static int
> -ovs_router_insert__(uint8_t priority, const struct in6_addr *ip6_dst,
> +ovs_router_insert__(uint32_t mark, uint8_t priority,
> +                    const struct in6_addr *ip6_dst,
>                     uint8_t plen, const char output_bridge[],
>                     const struct in6_addr *gw)
> {
> @@ -187,13 +198,14 @@ ovs_router_insert__(uint8_t priority, const struct 
> in6_addr *ip6_dst,
>     struct match match;
>     int err;
> 
> -    rt_init_match(&match, ip6_dst, plen);
> +    rt_init_match(&match, mark, ip6_dst, plen);
> 
>     p = xzalloc(sizeof *p);
>     ovs_strlcpy(p->output_bridge, output_bridge, sizeof p->output_bridge);
>     if (ipv6_addr_is_set(gw)) {
>         p->gw = *gw;
>     }
> +    p->mark = mark;
>     p->nw_addr = match.flow.ipv6_dst;
>     p->plen = plen;
>     p->priority = priority;
> @@ -202,7 +214,12 @@ ovs_router_insert__(uint8_t priority, const struct 
> in6_addr *ip6_dst,
>         err = get_src_addr(gw, output_bridge, &p->src_addr);
>     }
>     if (err) {
> +        struct ds ds = DS_EMPTY_INITIALIZER;
> +
> +        ipv6_format_mapped(ip6_dst, &ds);
> +        VLOG_DBG_RL(&rl, "src addr not available for route %s", 
> ds_cstr(&ds));
>         free(p);
> +        ds_destroy(&ds);
>         return err;
>     }
>     /* Longest prefix matches first. */
> @@ -222,13 +239,12 @@ ovs_router_insert__(uint8_t priority, const struct 
> in6_addr *ip6_dst,
> }
> 
> void
> -ovs_router_insert(const struct in6_addr *ip_dst, uint8_t plen,
> +ovs_router_insert(uint32_t mark, 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);
> +    ovs_router_insert__(mark, plen, ip_dst, plen, output_bridge, gw);
> }
> 
> -
> static bool
> __rt_entry_delete(const struct cls_rule *cr)
> {
> @@ -245,14 +261,15 @@ __rt_entry_delete(const struct cls_rule *cr)
> }
> 
> static bool
> -rt_entry_delete(uint8_t priority, const struct in6_addr *ip6_dst, uint8_t 
> plen)
> +rt_entry_delete(uint32_t mark, 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, ip6_dst, plen);
> +    rt_init_match(&match, mark, ip6_dst, plen);
> 
>     cls_rule_init(&rule, &match, priority);
> 
> @@ -292,32 +309,48 @@ static void
> ovs_router_add(struct unixctl_conn *conn, int argc,
>               const char *argv[], void *aux OVS_UNUSED)
> {
> -    ovs_be32 ip;
> -    unsigned int plen;
> +    struct in6_addr gw6 = in6addr_any;
>     struct in6_addr ip6;
> -    struct in6_addr gw6;
> +    uint32_t mark = 0;
> +    unsigned int plen;
> +    ovs_be32 ip;

Most of the above changes are not necessary.

>     int err;
> 
>     if (scan_ipv4_route(argv[1], &ip, &plen)) {
>         ovs_be32 gw = 0;
> -        if (argc > 3 && !ip_parse(argv[3], &gw)) {
> -            unixctl_command_reply_error(conn, "Invalid gateway");
> -            return;
> +
> +        if (argc > 3) {
> +            if (!ovs_scan(argv[3], "pkt_mark=%"SCNi32, &mark) &&
> +                !ip_parse(argv[3], &gw)) {
> +                unixctl_command_reply_error(conn, "Invalid pkt_mark or 
> gateway");
> +                return;
> +            }
>         }
>         in6_addr_set_mapped_ipv4(&ip6, ip);
> -        in6_addr_set_mapped_ipv4(&gw6, gw);
> +        if (gw) {
> +            in6_addr_set_mapped_ipv4(&gw6, gw);
> +        }
>         plen += 96;
>     } else if (scan_ipv6_route(argv[1], &ip6, &plen)) {
> -        gw6 = in6addr_any;
> -        if (argc > 3 && !ipv6_parse(argv[3], &gw6)) {
> -            unixctl_command_reply_error(conn, "Invalid IPv6 gateway");
> -            return;
> +        if (argc > 3) {
> +            if (!ovs_scan(argv[3], "pkt_mark=%"SCNi32, &mark) &&
> +                !ipv6_parse(argv[3], &gw6)) {
> +                unixctl_command_reply_error(conn, "Invalid pkt_mark or IPv6 
> gateway");
> +                return;
> +            }
>         }
>     } else {
>         unixctl_command_reply_error(conn, "Invalid parameters");
>         return;
>     }
> -    err = ovs_router_insert__(plen + 32, &ip6, plen, argv[2], &gw6);
> +    if (argc > 4) {
> +        if (!ovs_scan(argv[4], "pkt_mark=%"SCNi32, &mark)) {
> +            unixctl_command_reply_error(conn, "Invalid pkt_mark");
> +            return;
> +        }
> +    }
> +
> +    err = ovs_router_insert__(mark, plen + 32, &ip6, plen, argv[2], &gw6);
>     if (err) {
>         unixctl_command_reply_error(conn, "Error while inserting route.");
>     } else {
> @@ -329,9 +362,10 @@ static void
> ovs_router_del(struct unixctl_conn *conn, int argc OVS_UNUSED,
>               const char *argv[], void *aux OVS_UNUSED)
> {
> -    ovs_be32 ip;
> -    unsigned int plen;
>     struct in6_addr ip6;
> +    uint32_t mark = 0;
> +    unsigned int plen;
> +    ovs_be32 ip;
> 

Most of the changes above are not necessary.

>     if (scan_ipv4_route(argv[1], &ip, &plen)) {
>         in6_addr_set_mapped_ipv4(&ip6, ip);
> @@ -340,7 +374,14 @@ ovs_router_del(struct unixctl_conn *conn, int argc 
> OVS_UNUSED,
>         unixctl_command_reply_error(conn, "Invalid parameters");
>         return;
>     }
> -    if (rt_entry_delete(plen + 32, &ip6, plen)) {
> +    if (argc > 2) {
> +        if (!ovs_scan(argv[2], "pkt_mark=%"SCNi32, &mark)) {
> +            unixctl_command_reply_error(conn, "Invalid pkt_mark");
> +            return;
> +        }
> +    }
> +
> +    if (rt_entry_delete(mark, plen + 32, &ip6, plen)) {
>         unixctl_command_reply(conn, "OK");
>         seq_change(tnl_conf_seq);
>     } else {
> @@ -368,7 +409,12 @@ ovs_router_show(struct unixctl_conn *conn, int argc 
> OVS_UNUSED,
>         if (IN6_IS_ADDR_V4MAPPED(&rt->nw_addr)) {
>             plen -= 96;
>         }
> -        ds_put_format(&ds, "/%"PRIu16" dev %s", plen, rt->output_bridge);
> +        ds_put_format(&ds, "/%"PRIu16, plen);
> +        if (rt->mark) {
> +            ds_put_format(&ds, " MARK %"PRIu32, rt->mark);
> +        }
> +
> +        ds_put_format(&ds, " dev %s", rt->output_bridge);
>         if (ipv6_addr_is_set(&rt->gw)) {
>             ds_put_format(&ds, " GW ");
>             ipv6_format_mapped(&rt->gw, &ds);
> @@ -382,14 +428,15 @@ ovs_router_show(struct unixctl_conn *conn, int argc 
> OVS_UNUSED,
> }
> 
> static void
> -ovs_router_lookup_cmd(struct unixctl_conn *conn, int argc OVS_UNUSED,
> +ovs_router_lookup_cmd(struct unixctl_conn *conn, int argc,
>                       const char *argv[], void *aux OVS_UNUSED)
> {
> -    ovs_be32 ip;
> +    struct in6_addr gw, src;
> +    char iface[IFNAMSIZ];
>     struct in6_addr ip6;
>     unsigned int plen;
> -    char iface[IFNAMSIZ];
> -    struct in6_addr gw, src;
> +    uint32_t mark = 0;
> +    ovs_be32 ip;
> 

Most of the changes above are not necessary.

>     if (scan_ipv4_route(argv[1], &ip, &plen) && plen == 32) {
>         in6_addr_set_mapped_ipv4(&ip6, ip);
> @@ -397,9 +444,15 @@ ovs_router_lookup_cmd(struct unixctl_conn *conn, int 
> argc OVS_UNUSED,
>         unixctl_command_reply_error(conn, "Invalid parameters");
>         return;
>     }
> -
> -    if (ovs_router_lookup(&ip6, iface, &src, &gw)) {
> +    if (argc > 2) {
> +        if (!ovs_scan(argv[2], "pkt_mark=%"SCNi32, &mark)) {
> +            unixctl_command_reply_error(conn, "Invalid pkt_mark");
> +            return;
> +        }
> +    }
> +    if (ovs_router_lookup(mark, &ip6, iface, &src, &gw)) {
>         struct ds ds = DS_EMPTY_INITIALIZER;
> +
>         ds_put_format(&ds, "src ");
>         ipv6_format_mapped(&src, &ds);
>         ds_put_format(&ds, "\ngateway ");
> @@ -434,11 +487,11 @@ void
> ovs_router_init(void)
> {
>     classifier_init(&cls, NULL);
> -    unixctl_command_register("ovs/route/add", "ip_addr/prefix_len 
> out_br_name gw", 2, 3,
> +    unixctl_command_register("ovs/route/add", "ip_addr/prefix_len 
> out_br_name [gw | pkt_mark=mark]", 2, 4,

The above reads like you could optionally have ‘gw’ or ‘pkt_mark’. Should that 
be “[gw] [pkt_mark=mark]” instead?

>                              ovs_router_add, NULL);
>     unixctl_command_register("ovs/route/show", "", 0, 0, ovs_router_show, 
> NULL);
> -    unixctl_command_register("ovs/route/del", "ip_addr/prefix_len", 1, 1, 
> ovs_router_del,
> -                             NULL);
> -    unixctl_command_register("ovs/route/lookup", "ip_addr", 1, 1,
> +    unixctl_command_register("ovs/route/del", "ip_addr/prefix_len 
> [pkt_mark=mark]", 1, 2,
> +                             ovs_router_del, NULL);
> +    unixctl_command_register("ovs/route/lookup", "ip_addr [pkt_mark=mark]", 
> 1, 2,
>                              ovs_router_lookup_cmd, NULL);
> }
> diff --git a/lib/ovs-router.h b/lib/ovs-router.h
> index 9cb7509..29c7c5f 100644
> --- a/lib/ovs-router.h
> +++ b/lib/ovs-router.h
> @@ -25,10 +25,12 @@
> extern "C" {
> #endif
> 
> -bool ovs_router_lookup(const struct in6_addr *ip_dst, char out_dev[],
> +bool ovs_router_lookup(uint32_t mark, const struct in6_addr *ip_dst,
> +                       char out_dev[],
>                        struct in6_addr *src, struct in6_addr *gw);
> void ovs_router_init(void);
> -void ovs_router_insert(const struct in6_addr *ip_dst, uint8_t plen,
> +void ovs_router_insert(uint32_t mark, const struct in6_addr *ip_dst,
> +                       uint8_t plen,
>                        const char output_bridge[], const struct in6_addr *gw);
> void ovs_router_flush(void);
> #ifdef  __cplusplus
> diff --git a/lib/route-table.c b/lib/route-table.c
> index 61c8cd8..ae8af43 100644
> --- a/lib/route-table.c
> +++ b/lib/route-table.c
> @@ -298,7 +298,7 @@ route_table_handle_msg(const struct route_table_msg 
> *change)
>     if (change->relevant && change->nlmsg_type == RTM_NEWROUTE) {
>         const struct route_data *rd = &change->rd;
> 
> -        ovs_router_insert(&rd->rta_dst, rd->rtm_dst_len,
> +        ovs_router_insert(rd->mark, &rd->rta_dst, rd->rtm_dst_len,
>                           rd->ifname, &rd->rta_gw);
>     }
> }
> diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
> index e4ae760..520b8dd 100644
> --- a/ofproto/ofproto-dpif-sflow.c
> +++ b/ofproto/ofproto-dpif-sflow.c
> @@ -449,7 +449,10 @@ sflow_choose_agent_address(const char *agent_device,
>             struct in6_addr addr6, src, gw;
> 
>             in6_addr_set_mapped_ipv4(&addr6, sa.sin.sin_addr.s_addr);
> -            if (ovs_router_lookup(&addr6, name, &src, &gw)) {
> +            /* sFlow only supports target in default routing table with
> +             * packet mark zero.
> +             */
> +            if (ovs_router_lookup(0, &addr6, name, &src, &gw)) {
> 
>                 in4.s_addr = in6_addr_get_mapped_ipv4(&src);
>                 goto success;
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 0513394..f12acf3 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -2878,7 +2878,7 @@ tnl_route_lookup_flow(const struct flow *oflow,
>     struct in6_addr dst;
> 
>     dst = flow_tnl_dst(&oflow->tunnel);
> -    if (!ovs_router_lookup(&dst, out_dev, src, &gw)) {
> +    if (!ovs_router_lookup(oflow->pkt_mark, &dst, out_dev, src, &gw)) {
>         return -ENOENT;
>     }
> 
> diff --git a/tests/ovs-router.at b/tests/ovs-router.at
> index 93a730a..e1fa640 100644
> --- a/tests/ovs-router.at
> +++ b/tests/ovs-router.at
> @@ -20,10 +20,32 @@ AT_CHECK([ovs-appctl netdev-dummy/ip4addr br0 
> 192.0.2.1/24], [0], [OK
> ])
> AT_CHECK([ovs-appctl ovs/route/add 198.51.100.0/24 br0 192.0.2.254], [0], [OK
> ])
> +AT_CHECK([ovs-appctl ovs/route/add 192.0.2.1/24 br0 pkt_mark=123], [0], [OK
> +])
> +
> +AT_CHECK([ovs-appctl ovs/route/add 198.51.100.200/24 br0 192.0.2.250 
> pkt_mark=1234], [0], [OK
> +])
> +
> +AT_CHECK([ovs-appctl ovs/route/show | grep User | sort], [0], [User: 
> 192.0.2.0/24 MARK 123 dev br0 SRC 192.0.2.1
> +User: 198.51.100.0/24 MARK 1234 dev br0 GW 192.0.2.250 SRC 192.0.2.1
> +User: 198.51.100.0/24 dev br0 GW 192.0.2.254 SRC 192.0.2.1
> +])
> +
> AT_CHECK([ovs-appctl ovs/route/lookup 198.51.100.1], [0], [src 192.0.2.1
> gateway 192.0.2.254
> dev br0
> ])
> +
> +AT_CHECK([ovs-appctl ovs/route/lookup 198.51.100.1 pkt_mark=1234], [0], [src 
> 192.0.2.1
> +gateway 192.0.2.250
> +dev br0
> +])
> +AT_CHECK([ovs-appctl ovs/route/del 198.51.100.0/24 pkt_mark=1234], [0], [OK
> +])
> +AT_CHECK([ovs-appctl ovs/route/show | grep User | sort], [0], [User: 
> 192.0.2.0/24 MARK 123 dev br0 SRC 192.0.2.1
> +User: 198.51.100.0/24 dev br0 GW 192.0.2.254 SRC 192.0.2.1
> +])
> +
> OVS_VSWITCHD_STOP
> AT_CLEANUP
> 
> @@ -34,9 +56,29 @@ AT_CHECK([ovs-appctl netdev-dummy/ip6addr br0 
> 2001:db8:cafe::1/64], [0], [OK
> ])
> AT_CHECK([ovs-appctl ovs/route/add 2001:db8:babe::/64 br0 2001:db8:cafe::2], 
> [0], [OK
> ])
> +AT_CHECK([ovs-appctl ovs/route/add 2001:db8:babe::/64 br0 2001:db8:cafe::3 
> pkt_mark=321], [0], [OK
> +])
> +
> +AT_CHECK([ovs-appctl ovs/route/show | grep User | sort], [0], [dnl
> +User: 2001:db8:babe::/64 MARK 321 dev br0 GW 2001:db8:cafe::3 SRC 
> 2001:db8:cafe::1
> +User: 2001:db8:babe::/64 dev br0 GW 2001:db8:cafe::2 SRC 2001:db8:cafe::1
> +])
> +
> AT_CHECK([ovs-appctl ovs/route/lookup 2001:db8:babe::1eaf], [0], [src 
> 2001:db8:cafe::1
> gateway 2001:db8:cafe::2
> dev br0
> ])
> +
> +AT_CHECK([ovs-appctl ovs/route/lookup 2001:db8:babe::1eaf pkt_mark=321], 
> [0], [src 2001:db8:cafe::1
> +gateway 2001:db8:cafe::3
> +dev br0
> +])
> +
> +AT_CHECK([ovs-appctl ovs/route/del 2001:db8:babe::/64 pkt_mark=321], [0], [OK
> +])
> +AT_CHECK([ovs-appctl ovs/route/show | grep User | sort], [0], [dnl
> +User: 2001:db8:babe::/64 dev br0 GW 2001:db8:cafe::2 SRC 2001:db8:cafe::1
> +])
> +
> OVS_VSWITCHD_STOP
> AT_CLEANUP
> diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at
> index 4aaa669..4eeac41 100644
> --- a/tests/tunnel-push-pop.at
> +++ b/tests/tunnel-push-pop.at
> @@ -40,6 +40,9 @@ AT_CHECK([ovs-appctl netdev-dummy/ip6addr br0 
> 2001:cafe::88/24], [0], [OK
> AT_CHECK([ovs-appctl ovs/route/add 1.1.2.92/24 br0], [0], [OK
> ])
> 
> +AT_CHECK([ovs-appctl ovs/route/add 1.1.2.92/24 br0 pkt_mark=1234], [0], [OK
> +])
> +
> AT_CHECK([ovs-ofctl add-flow br0 action=normal])
> 
> dnl Check ARP request
> -- 
> 2.9.3
> 
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to