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 <al...@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 <al...@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@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
