[ovs-dev] [PATCH 2/2] bfd: Allow users to set local/remote src/dst MAC address.

Ethan Jackson ethan at nicira.com
Sun Aug 10 22:11:02 UTC 2014


For the local eth_src and eth_dst, instead of having the various is
set booleans, wouldn't it be simpler to just copy the default value
into the struct bfd in bfd_configure() instead of doing it in
bfd_put_packet()?

I'm not sure we should have the rmt_eth_src flag.  We should probably
accept bfd packets with the appropriate eth_dst no matter what the
eth_src is.

Ethan

On Fri, Aug 8, 2014 at 5:02 PM, Alex Wang <alexw at nicira.com> wrote:
> This commit adds four options for configuring the MAC addresses
> in BFD state machine.  Therein, the "bfd_local_src_mac" and
> "bfd_local_dst_mac" configure the MAC address of sent BFD
> packets.  The "bfd_remote_src_mac" and "bfd_remote_dst_mac"
> configure the matching of MAC address on recevied BFD packets.
>
> Signed-off-by: Alex Wang <alexw at nicira.com>
> ---
>  lib/bfd.c            |   60 +++++++++++++++++++++++++++++++++++++++-----------
>  vswitchd/vswitch.xml |   32 +++++++++++++++++++++++----
>  2 files changed, 75 insertions(+), 17 deletions(-)
>
> diff --git a/lib/bfd.c b/lib/bfd.c
> index 9069582..165eb61 100644
> --- a/lib/bfd.c
> +++ b/lib/bfd.c
> @@ -169,8 +169,15 @@ struct bfd {
>
>      uint32_t rmt_disc;            /* bfd.RemoteDiscr. */
>
> -    uint8_t eth_dst[ETH_ADDR_LEN];/* Ethernet destination address. */
> -    bool eth_dst_set;             /* 'eth_dst' set through database. */
> +    uint8_t local_eth_src[ETH_ADDR_LEN]; /* Local eth src address. */
> +    bool local_eth_src_set;              /* Local 'eth_src' set through db. */
> +    uint8_t local_eth_dst[ETH_ADDR_LEN]; /* Local eth dst address. */
> +    bool local_eth_dst_set;              /* Local 'eth_dst' set through db. */
> +
> +    uint8_t rmt_eth_src[ETH_ADDR_LEN];   /* Remote eth src address. */
> +    bool rmt_eth_src_set;                /* Remote 'eth_src' set through db. */
> +    uint8_t rmt_eth_dst[ETH_ADDR_LEN];   /* Remote eth dst address. */
> +    bool rmt_eth_dst_set;                /* Remote 'eth_dst' set through db. */
>
>      ovs_be32 ip_src;              /* IPv4 source address. */
>      ovs_be32 ip_dst;              /* IPv4 destination address. */
> @@ -388,8 +395,6 @@ bfd_configure(struct bfd *bfd, const char *name, const struct smap *cfg,
>
>          bfd_set_state(bfd, STATE_DOWN, DIAG_NONE);
>
> -        memcpy(bfd->eth_dst, eth_addr_bfd, ETH_ADDR_LEN);
> -
>          bfd_status_changed(bfd);
>      }
>
> @@ -440,13 +445,36 @@ bfd_configure(struct bfd *bfd, const char *name, const struct smap *cfg,
>          need_poll = true;
>      }
>
> -    hwaddr = smap_get(cfg, "bfd_dst_mac");
> +    hwaddr = smap_get(cfg, "bfd_local_src_mac");
> +    if (hwaddr && eth_addr_from_string(hwaddr, ea) && !eth_addr_is_zero(ea)) {
> +        memcpy(bfd->local_eth_src, ea, ETH_ADDR_LEN);
> +        bfd->local_eth_src_set = true;
> +    } else {
> +        bfd->local_eth_src_set = false;
> +    }
> +
> +    hwaddr = smap_get(cfg, "bfd_local_dst_mac");
>      if (hwaddr && eth_addr_from_string(hwaddr, ea) && !eth_addr_is_zero(ea)) {
> -        memcpy(bfd->eth_dst, ea, ETH_ADDR_LEN);
> -        bfd->eth_dst_set = true;
> -    } else if (bfd->eth_dst_set) {
> -        memcpy(bfd->eth_dst, eth_addr_bfd, ETH_ADDR_LEN);
> -        bfd->eth_dst_set = false;
> +        memcpy(bfd->local_eth_dst, ea, ETH_ADDR_LEN);
> +        bfd->local_eth_dst_set = true;
> +    } else {
> +        bfd->local_eth_dst_set = false;
> +    }
> +
> +    hwaddr = smap_get(cfg, "bfd_remote_src_mac");
> +    if (hwaddr && eth_addr_from_string(hwaddr, ea) && !eth_addr_is_zero(ea)) {
> +        memcpy(bfd->rmt_eth_src, ea, ETH_ADDR_LEN);
> +        bfd->rmt_eth_src_set = true;
> +    } else {
> +        bfd->rmt_eth_src_set = false;
> +    }
> +
> +    hwaddr = smap_get(cfg, "bfd_remote_dst_mac");
> +    if (hwaddr && eth_addr_from_string(hwaddr, ea) && !eth_addr_is_zero(ea)) {
> +        memcpy(bfd->rmt_eth_dst, ea, ETH_ADDR_LEN);
> +        bfd->rmt_eth_dst_set = true;
> +    } else {
> +        bfd->rmt_eth_dst_set = false;
>      }
>
>      ip_src = smap_get(cfg, "bfd_src_ip");
> @@ -600,8 +628,10 @@ bfd_put_packet(struct bfd *bfd, struct ofpbuf *p,
>
>      ofpbuf_reserve(p, 2); /* Properly align after the ethernet header. */
>      eth = ofpbuf_put_uninit(p, sizeof *eth);
> -    memcpy(eth->eth_src, eth_src, ETH_ADDR_LEN);
> -    memcpy(eth->eth_dst, bfd->eth_dst, ETH_ADDR_LEN);
> +    memcpy(eth->eth_src, bfd->local_eth_src_set ? bfd->local_eth_src : eth_src,
> +           ETH_ADDR_LEN);
> +    memcpy(eth->eth_dst, bfd->local_eth_dst_set ? bfd->local_eth_dst
> +                                                : eth_addr_bfd, ETH_ADDR_LEN);
>      eth->eth_type = htons(ETH_TYPE_IP);
>
>      ip = ofpbuf_put_zeros(p, sizeof *ip);
> @@ -656,8 +686,12 @@ bfd_should_process_flow(const struct bfd *bfd_, const struct flow *flow,
>      struct bfd *bfd = CONST_CAST(struct bfd *, bfd_);
>      bool check_tnl_key;
>
> +    memset(&wc->masks.dl_src, 0xff, sizeof wc->masks.dl_src);
>      memset(&wc->masks.dl_dst, 0xff, sizeof wc->masks.dl_dst);
> -    if (bfd->eth_dst_set && memcmp(bfd->eth_dst, flow->dl_dst, ETH_ADDR_LEN)) {
> +    if ((bfd->rmt_eth_src_set && memcmp(bfd->rmt_eth_src, flow->dl_src,
> +                                        ETH_ADDR_LEN))
> +        || (bfd->rmt_eth_dst_set && memcmp(bfd->rmt_eth_dst, flow->dl_dst,
> +                                           ETH_ADDR_LEN))) {
>          return false;
>      }
>
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index bff8fb6..07b696a 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -2081,12 +2081,36 @@
>            tunnel key.
>         </column>
>
> -       <column name="bfd" key="bfd_dst_mac">
> +       <column name="bfd" key="bfd_local_src_mac">
>           Set to an Ethernet address in the form
>           <var>xx</var>:<var>xx</var>:<var>xx</var>:<var>xx</var>:<var>xx</var>:<var>xx</var>
> -         to set the MAC used as destination for transmitted BFD packets and
> -         expected as destination for received BFD packets.  The default is
> -         <code>00:23:20:00:00:01</code>.
> +         to set the MAC used as source for transmitted BFD packets.  The
> +         default is the mac address of the BFD enabled interface.
> +       </column>
> +
> +       <column name="bfd" key="bfd_local_dst_mac">
> +         Set to an Ethernet address in the form
> +         <var>xx</var>:<var>xx</var>:<var>xx</var>:<var>xx</var>:<var>xx</var>:<var>xx</var>
> +         to set the MAC used as destination for transmitted BFD packets.  The
> +         default is <code>00:23:20:00:00:01</code>.
> +       </column>
> +
> +       <column name="bfd" key="bfd_remote_src_mac">
> +         Set to an Ethernet address in the form
> +         <var>xx</var>:<var>xx</var>:<var>xx</var>:<var>xx</var>:<var>xx</var>:<var>xx</var>
> +         to set the MAC used for checking the source of received BFD packets.
> +         Packets with different source MAC will not be considered as BFD packets.
> +         If not specified the destination MAC address of received BFD packets
> +         are not checked.
> +       </column>
> +
> +       <column name="bfd" key="bfd_remoe_dst_mac">
> +         Set to an Ethernet address in the form
> +         <var>xx</var>:<var>xx</var>:<var>xx</var>:<var>xx</var>:<var>xx</var>:<var>xx</var>
> +         to set the MAC used for checking the destination of received BFD packets.
> +         Packets with different destination MAC will not be considered as BFD packets.
> +         If not specified the destination MAC address of received BFD packets
> +         are not checked.
>         </column>
>
>         <column name="bfd" key="bfd_src_ip">
> --
> 1.7.9.5
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list