From b898326e4a15199d5b8555d158a3509bd2bfcb8b Mon Sep 17 00:00:00 2001 From: Divakar Date: Sat, 27 Feb 2016 09:37:41 +0530 Subject: [PATCH] Use innerpacket's destip as source ip while doing Tx Port mirroring When Transmit port mirroring is enabled, packet received on Fabric interface is right now mirrored using the source IP of the inner packet. This results in RPF failure on Analyzer VM's compute node because the compute node which is doing the port mirroring is using other compute node's VM IP. As a fix, if mirroring is Tax mirroring, rather using inner packets source ip, dest ip is used, so that Analyzer VM's RPF will not have any issues Using the correct FMD and setting pkt_type correctly for mirrored packets If port mirroring is enabled, cloned packet is subjected to mirroring using the original packet's forwarding metadata. If mirroring code changes the metadata content, original packet will be forwarded as per the changed fmd and results in wrong forwarding. In the current case, mirroring is to a VM in different VN (hence new VRF) and mirroring code is modifying the fmd's dvrf to new VRF. The original ARP packet's L2 and L3 looksups are happening on the modified VRF resulting in ARP getting dropped. Also the type of packet is identified using vr_pkt_type() after packet is mirrored. This is resulting in wrong pkt_type being used for mirrored packet hence the source IP packet the mirrored packet is not correctly computed. As a fix, new FMD is used for mirrored packet and packet type is identified before mirroring itself. closes-bug: #1549727 closes-bug: #1550312 Change-Id: If74cbbd2d80dbd9c95f674414cadc86ba43b3782 --- dp-core/vr_datapath.c | 10 ++++----- dp-core/vr_mirror.c | 51 ++++++++++++++++++++++++------------------- dp-core/vr_nexthop.c | 8 +++++++ 3 files changed, 42 insertions(+), 27 deletions(-) diff --git a/dp-core/vr_datapath.c b/dp-core/vr_datapath.c index 2188f2e6e..1772c4c93 100644 --- a/dp-core/vr_datapath.c +++ b/dp-core/vr_datapath.c @@ -569,17 +569,17 @@ vr_virtual_input(unsigned short vrf, struct vr_interface *vif, fmd.fmd_vlan = vlan_id; fmd.fmd_dvrf = vrf; + if (vr_pkt_type(pkt, 0, &fmd) < 0) { + vif_drop_pkt(vif, pkt, 1); + return 0; + } + if (vif->vif_flags & VIF_FLAG_MIRROR_RX) { mfmd = fmd; mfmd.fmd_dvrf = vif->vif_vrf; vr_mirror(vif->vif_router, vif->vif_mirror_id, pkt, &mfmd); } - if (vr_pkt_type(pkt, 0, &fmd) < 0) { - vif_drop_pkt(vif, pkt, 1); - return 0; - } - /* * we really do not allow any broadcast packets from interfaces * that are part of transparent service chain, since transparent diff --git a/dp-core/vr_mirror.c b/dp-core/vr_mirror.c index 1f9e92fe2..1babcea0e 100644 --- a/dp-core/vr_mirror.c +++ b/dp-core/vr_mirror.c @@ -368,22 +368,27 @@ int vr_mirror(struct vrouter *router, uint8_t mirror_id, struct vr_packet *pkt, struct vr_forwarding_md *fmd) { + bool reset = true; + unsigned int captured_len, clone_len = VR_MIRROR_PKT_HEAD_SPACE, + mirror_md_len = 0; + unsigned char default_mme[2] = {0xff, 0x0}; + void *mirror_md; unsigned char *buf; - struct vr_nexthop *nh; + struct vr_nexthop *nh, *pkt_nh; struct vr_pcap *pcap; struct vr_mirror_entry *mirror; struct vr_mirror_meta_entry *mme; - unsigned int captured_len, clone_len = VR_MIRROR_PKT_HEAD_SPACE; - unsigned int mirror_md_len = 0; - unsigned char default_mme[2] = {0xff, 0x0}; - void *mirror_md; - struct vr_nexthop *pkt_nh; - bool reset; + struct vr_forwarding_md new_fmd; mirror = router->vr_mirrors[mirror_id]; if (!mirror) return 0; + if (fmd) { + memcpy(&new_fmd, fmd, sizeof(*fmd)); + fmd = &new_fmd; + } + if (fmd->fmd_flow_index >= 0) { mme = (struct vr_mirror_meta_entry *)vr_itable_get(router->vr_mirror_md, fmd->fmd_flow_index); @@ -407,35 +412,37 @@ vr_mirror(struct vrouter *router, uint8_t mirror_id, * header. If not get the processed headers by resetting the packet * and mirror it */ - reset = true; - if (pkt->vp_if && pkt->vp_if->vif_type == VIF_TYPE_PHYSICAL) { + if (pkt->vp_if && (pkt->vp_if->vif_type == VIF_TYPE_PHYSICAL)) { pkt_nh = pkt->vp_nh; if (pkt_nh && (pkt_nh->nh_flags & NH_FLAG_VALID) && (pkt_nh->nh_type == NH_ENCAP)) { reset = false; - if (pkt_nh->nh_family == AF_INET) - clone_len += pkt_nh->nh_encap_len; - - if (vr_pcow(pkt, clone_len)) - goto fail; - - - if (pkt_nh->nh_family == AF_INET) { - if (!pkt_nh->nh_dev->vif_set_rewrite(pkt_nh->nh_dev, pkt, fmd, - pkt_nh->nh_data, pkt_nh->nh_encap_len)) - goto fail; + if (fmd->fmd_flow_index >= 0) { + if (pkt_nh->nh_family == AF_INET) + clone_len += pkt_nh->nh_encap_len; + + if (vr_pcow(pkt, clone_len)) + goto fail; + clone_len = 0; + + if (pkt_nh->nh_family == AF_INET) { + if (!pkt_nh->nh_dev->vif_set_rewrite(pkt_nh->nh_dev, pkt, fmd, + pkt_nh->nh_data, pkt_nh->nh_encap_len)) + goto fail; + } } } } - if (reset) { + if (reset) vr_preset(pkt); + + if (clone_len) { if (vr_pcow(pkt, clone_len)) goto fail; } - pkt->vp_flags |= VP_FLAG_FROM_DP; /* Set the GSO and partial checksum flag */ pkt->vp_flags |= (VP_FLAG_FLOW_SET | VP_FLAG_GSO | VP_FLAG_CSUM_PARTIAL); diff --git a/dp-core/vr_nexthop.c b/dp-core/vr_nexthop.c index 5849ec906..668587839 100644 --- a/dp-core/vr_nexthop.c +++ b/dp-core/vr_nexthop.c @@ -1236,6 +1236,14 @@ nh_generate_sip(struct vr_nexthop *nh, struct vr_packet *pkt) iph = (struct vr_ip *)pkt_network_header(pkt); if (pkt->vp_type == VP_TYPE_IP) { + + /* + * If the packet is from fabric, it must be destined to a VM on + * this compute, so lets use dest ip + */ + if (pkt->vp_if->vif_type == VIF_TYPE_PHYSICAL) + return iph->ip_daddr; + return iph->ip_saddr; }