From b7cc0d7cd984d89b382b163d5b636a88814288f5 Mon Sep 17 00:00:00 2001 From: Manish Date: Fri, 6 May 2016 00:51:14 +0530 Subject: [PATCH] In BGP service if VM sends TTL 1 session doesnt come up. If TTL is sent as 1 by VM, modify it to 255 and for reverse keep it at 2. (Logic for 2 is that vrouter decrements TTL because of routing and hence VM should get packet back with 1). If TTL is anything else than 1, then honor it. Closes-bug: #1567586 Conflicts: src/vnsw/agent/pkt/flow_entry.cc src/vnsw/agent/pkt/pkt_flow_info.h src/vnsw/agent/vrouter/ksync/flowtable_ksync.cc Change-Id: Ie2f5e5c3513d2e1a1b26d0c47f906a3a70f32abb --- src/vnsw/agent/pkt/flow_entry.cc | 13 ++++++ src/vnsw/agent/pkt/flow_entry.h | 1 + src/vnsw/agent/pkt/pkt_flow_info.cc | 1 + src/vnsw/agent/pkt/pkt_flow_info.h | 4 +- src/vnsw/agent/pkt/pkt_handler.cc | 14 +++--- src/vnsw/agent/pkt/pkt_handler.h | 4 ++ src/vnsw/agent/pkt/test/test_bgp_service.cc | 44 +++++++++++++++++++ src/vnsw/agent/pkt/test/test_pkt_util.cc | 9 ++-- src/vnsw/agent/pkt/test/test_pkt_util.h | 4 +- src/vnsw/agent/test/pkt_gen.h | 3 +- .../agent/vrouter/ksync/flowtable_ksync.cc | 2 + 11 files changed, 85 insertions(+), 14 deletions(-) diff --git a/src/vnsw/agent/pkt/flow_entry.cc b/src/vnsw/agent/pkt/flow_entry.cc index 7d5b77b3e86..39af8f983ce 100644 --- a/src/vnsw/agent/pkt/flow_entry.cc +++ b/src/vnsw/agent/pkt/flow_entry.cc @@ -286,6 +286,7 @@ void FlowData::Reset() { ecmp_rpf_nh_ = 0; acl_assigned_vrf_index_ = VrfEntry::kInvalidIndex; qos_config_idx = AgentQosConfigTable::kInvalidIndex; + ttl = 0; } static std::vector MakeList(const VnListType &ilist) { @@ -581,6 +582,12 @@ void FlowEntry::InitFwdFlow(const PktFlowInfo *info, const PktInfo *pkt, SetRpfNH(flow_table_, ctrl->rt_); } + if (info->bgp_router_service_flow) { + if (info->ttl == 1) { + data_.ttl = BGP_SERVICE_TTL_FWD_FLOW; + } + } + data_.flow_source_vrf = info->flow_source_vrf; data_.flow_dest_vrf = info->flow_dest_vrf; data_.flow_source_plen_map = info->flow_source_plen_map; @@ -651,6 +658,12 @@ void FlowEntry::InitRevFlow(const PktFlowInfo *info, const PktInfo *pkt, SetRpfNH(flow_table_, ctrl->rt_); } + if (info->bgp_router_service_flow) { + if (info->ttl == 1) { + data_.ttl = BGP_SERVICE_TTL_REV_FLOW; + } + } + data_.flow_source_vrf = info->flow_dest_vrf; data_.flow_dest_vrf = info->flow_source_vrf; data_.flow_source_plen_map = info->flow_dest_plen_map; diff --git a/src/vnsw/agent/pkt/flow_entry.h b/src/vnsw/agent/pkt/flow_entry.h index 859a936a9e1..c7077c26cb2 100644 --- a/src/vnsw/agent/pkt/flow_entry.h +++ b/src/vnsw/agent/pkt/flow_entry.h @@ -280,6 +280,7 @@ struct FlowData { uint32_t dest_vrf; uint32_t component_nh_idx; uint32_t bgp_as_a_service_port; + uint32_t ttl; // Stats uint8_t source_plen; diff --git a/src/vnsw/agent/pkt/pkt_flow_info.cc b/src/vnsw/agent/pkt/pkt_flow_info.cc index 48bbd526387..aaa9a759b1f 100644 --- a/src/vnsw/agent/pkt/pkt_flow_info.cc +++ b/src/vnsw/agent/pkt/pkt_flow_info.cc @@ -822,6 +822,7 @@ void PktFlowInfo::BgpRouterServiceFromVm(const PktInfo *pkt, PktControlInfo *in, out->rt_ = FlowEntry::GetUcRoute(out->vrf_, nat_server); out->intf_ = agent->vhost_interface(); out->nh_ = out->intf_->flow_key_nh()->id(); + ttl = pkt->ttl; return; } diff --git a/src/vnsw/agent/pkt/pkt_flow_info.h b/src/vnsw/agent/pkt/pkt_flow_info.h index be06ac987c7..4f5db8a68a6 100644 --- a/src/vnsw/agent/pkt/pkt_flow_info.h +++ b/src/vnsw/agent/pkt/pkt_flow_info.h @@ -54,7 +54,7 @@ class PktFlowInfo { trap_rev_flow(false), fip_snat(false), fip_dnat(false), snat_fip(), short_flow_reason(0), peer_vrouter(), tunnel_type(TunnelType::INVALID), flood_unknown_unicast(false), bgp_router_service_flow(false), - alias_ip_flow(false) { + alias_ip_flow(false), ttl(0) { } static bool ComputeDirection(const Interface *intf); @@ -190,6 +190,8 @@ class PktFlowInfo { // Alias IP flow bool alias_ip_flow; + //TTL of nat'd flow especially bgp-service flows + uint8_t ttl; }; #endif // __agent_pkt_flow_info_h_ diff --git a/src/vnsw/agent/pkt/pkt_handler.cc b/src/vnsw/agent/pkt/pkt_handler.cc index 61827d27dd8..af6fecf8303 100644 --- a/src/vnsw/agent/pkt/pkt_handler.cc +++ b/src/vnsw/agent/pkt/pkt_handler.cc @@ -376,6 +376,7 @@ int PktHandler::ParseIpPacket(PktInfo *pkt_info, PktType::Type &pkt_type, pkt_info->ip_saddr = IpAddress(Ip4Address(ntohl(ip->ip_src.s_addr))); pkt_info->ip_daddr = IpAddress(Ip4Address(ntohl(ip->ip_dst.s_addr))); pkt_info->ip_proto = ip->ip_p; + pkt_info->ttl = ip->ip_ttl; len += (ip->ip_hl << 2); } else if (pkt_info->ether_type == ETHERTYPE_IPV6) { pkt_info->family = Address::INET6; @@ -393,6 +394,7 @@ int PktHandler::ParseIpPacket(PktInfo *pkt_info, PktType::Type &pkt_type, addr[i] = ip->ip6_dst.s6_addr[i]; } pkt_info->ip_daddr = IpAddress(Ip6Address(addr)); + pkt_info->ttl = ip->ip6_hlim; // Look for known transport headers. Fallback to the last header if // no known header is found @@ -975,8 +977,8 @@ PktInfo::PktInfo(const PacketBufferPtr &buff) : pkt(buff->data()), len(buff->data_len()), max_pkt_len(buff->buffer_len()), data(), ipc(), family(Address::UNSPEC), type(PktType::INVALID), agent_hdr(), ether_type(-1), ip_saddr(), ip_daddr(), ip_proto(), sport(), dport(), - tcp_ack(false), tunnel(), l3_forwarding(false), l3_label(false), eth(), - arp(), ip(), ip6(), packet_buffer_(buff) { + ttl(0), tcp_ack(false), tunnel(), l3_forwarding(false), + l3_label(false), eth(), arp(), ip(), ip6(), packet_buffer_(buff) { transp.tcp = 0; } @@ -984,8 +986,8 @@ PktInfo::PktInfo(const PacketBufferPtr &buff, const AgentHdr &hdr) : pkt(buff->data()), len(buff->data_len()), max_pkt_len(buff->buffer_len()), data(), ipc(), family(Address::UNSPEC), type(PktType::INVALID), agent_hdr(hdr), ether_type(-1), ip_saddr(), ip_daddr(), ip_proto(), sport(), - dport(), tcp_ack(false), tunnel(), l3_forwarding(false), l3_label(false), - eth(), arp(), ip(), ip6(), packet_buffer_(buff) { + dport(), ttl(0), tcp_ack(false), tunnel(), l3_forwarding(false), + l3_label(false), eth(), arp(), ip(), ip6(), packet_buffer_(buff) { transp.tcp = 0; } @@ -994,7 +996,7 @@ PktInfo::PktInfo(Agent *agent, uint32_t buff_len, PktHandler::PktModuleName mod, module(mod), len(), max_pkt_len(), data(), ipc(), family(Address::UNSPEC), type(PktType::INVALID), agent_hdr(), ether_type(-1), ip_saddr(), ip_daddr(), - ip_proto(), sport(), dport(), tcp_ack(false), tunnel(), + ip_proto(), sport(), dport(), ttl(0), tcp_ack(false), tunnel(), l3_forwarding(false), l3_label(false), eth(), arp(), ip(), ip6() { packet_buffer_ = agent->pkt()->packet_buffer_manager()->Allocate @@ -1010,7 +1012,7 @@ PktInfo::PktInfo(PktHandler::PktModuleName mod, InterTaskMsg *msg) : module(mod), pkt(), len(), max_pkt_len(0), data(), ipc(msg), family(Address::UNSPEC), type(PktType::MESSAGE), agent_hdr(), ether_type(-1), ip_saddr(), ip_daddr(), - ip_proto(), sport(), dport(), tcp_ack(false), tunnel(), + ip_proto(), sport(), dport(), ttl(0), tcp_ack(false), tunnel(), l3_forwarding(false), l3_label(false), eth(), arp(), ip(), ip6(), packet_buffer_() { transp.tcp = 0; } diff --git a/src/vnsw/agent/pkt/pkt_handler.h b/src/vnsw/agent/pkt/pkt_handler.h index 578350cae1a..5362cc9dbe5 100644 --- a/src/vnsw/agent/pkt/pkt_handler.h +++ b/src/vnsw/agent/pkt/pkt_handler.h @@ -38,6 +38,9 @@ #define IPC_HDR_LEN (sizeof(struct ether_header) + sizeof(struct agent_hdr)) #define VLAN_PROTOCOL 0x8100 #define DEFAULT_IP_TTL 64 +//Ideally VM is one hop away but traffic gets routed so use 2. +#define BGP_SERVICE_TTL_REV_FLOW 2 +#define BGP_SERVICE_TTL_FWD_FLOW 255 #define DEFAULT_IP_ID 0 #define VLAN_HDR_LEN 4 @@ -350,6 +353,7 @@ struct PktInfo { uint8_t ip_proto; uint32_t sport; uint32_t dport; + uint32_t ttl; bool tcp_ack; TunnelInfo tunnel; diff --git a/src/vnsw/agent/pkt/test/test_bgp_service.cc b/src/vnsw/agent/pkt/test/test_bgp_service.cc index a35a579e058..594f4b04189 100644 --- a/src/vnsw/agent/pkt/test/test_bgp_service.cc +++ b/src/vnsw/agent/pkt/test/test_bgp_service.cc @@ -7,6 +7,7 @@ #include "test_pkt_util.h" #include "pkt/flow_proto.h" #include "pkt/flow_mgmt.h" +#include "pkt/pkt_handler.h" #include #include @@ -83,6 +84,49 @@ class BgpServiceTest : public ::testing::Test { BgpPeer *peer; }; +//TTL 1 +TEST_F(BgpServiceTest, Test_ttl_1) { + AddAap("vnet1", 1, Ip4Address::from_string("10.10.10.10"), "00:00:01:01:01:01"); + peer = CreateBgpPeer("127.0.0.1", "remote"); + client->WaitForIdle(); + + TxTcpPacket(VmInterfaceGet(1)->id(), "10.10.10.10", "1.1.1.1", 10000, 179, + false, 1, 1, 1); + client->WaitForIdle(); + FlowEntry *fe = FlowGet(VmInterfaceGet(1)->flow_key_nh()->id(), + "10.10.10.10", "1.1.1.1", 6, 10000, 179); + EXPECT_TRUE(fe != NULL); + EXPECT_TRUE(fe->reverse_flow_entry() != NULL); + EXPECT_TRUE(fe->is_flags_set(FlowEntry::BgpRouterService)); + EXPECT_TRUE(fe->data().ttl == BGP_SERVICE_TTL_FWD_FLOW); + EXPECT_TRUE(fe->reverse_flow_entry()->data().ttl == + BGP_SERVICE_TTL_REV_FLOW); + + DeleteBgpPeer(peer); + client->WaitForIdle(); +} + +//TTL 64 +TEST_F(BgpServiceTest, Test_ttl_2) { + AddAap("vnet1", 1, Ip4Address::from_string("10.10.10.10"), "00:00:01:01:01:01"); + peer = CreateBgpPeer("127.0.0.1", "remote"); + client->WaitForIdle(); + + TxTcpPacket(VmInterfaceGet(1)->id(), "10.10.10.10", "1.1.1.1", 10000, 179, + false, 1, 1, 64); + client->WaitForIdle(); + FlowEntry *fe = FlowGet(VmInterfaceGet(1)->flow_key_nh()->id(), + "10.10.10.10", "1.1.1.1", 6, 10000, 179); + EXPECT_TRUE(fe != NULL); + EXPECT_TRUE(fe->reverse_flow_entry() != NULL); + EXPECT_TRUE(fe->is_flags_set(FlowEntry::BgpRouterService)); + EXPECT_TRUE(fe->data().ttl == 0); + EXPECT_TRUE(fe->reverse_flow_entry()->data().ttl == 0); + + DeleteBgpPeer(peer); + client->WaitForIdle(); +} + TEST_F(BgpServiceTest, Test_1) { peer = CreateBgpPeer("127.0.0.1", "remote"); client->WaitForIdle(); diff --git a/src/vnsw/agent/pkt/test/test_pkt_util.cc b/src/vnsw/agent/pkt/test/test_pkt_util.cc index 5e084c98b95..25a846e4089 100644 --- a/src/vnsw/agent/pkt/test/test_pkt_util.cc +++ b/src/vnsw/agent/pkt/test/test_pkt_util.cc @@ -101,20 +101,21 @@ void MakeSctpPacket(PktGen *pkt, int ifindex, const char *sip, void MakeTcpPacket(PktGen *pkt, int ifindex, const char *sip, const char *dip, uint16_t sport, uint16_t dport, bool ack, - int hash_id, uint32_t vrf_id) { + int hash_id, uint32_t vrf_id, int ttl) { pkt->AddEthHdr("00:00:00:00:00:01", "00:00:00:00:00:02", 0x800); pkt->AddAgentHdr(ifindex, AgentHdr::TRAP_FLOW_MISS, hash_id, vrf_id); pkt->AddEthHdr("00:00:5E:00:01:00", "00:00:00:00:00:01", 0x800); - pkt->AddIpHdr(sip, dip, IPPROTO_TCP); + pkt->AddIpHdr(sip, dip, IPPROTO_TCP, false, ttl); pkt->AddTcpHdr(sport, dport, false, false, ack, 64); } void TxTcpPacket(int ifindex, const char *sip, const char *dip, uint16_t sport, uint16_t dport, bool ack, int hash_id, - uint32_t vrf_id) { + uint32_t vrf_id, int ttl) { PktGen *pkt = new PktGen(); - MakeTcpPacket(pkt, ifindex, sip, dip, sport, dport, ack, hash_id, vrf_id); + MakeTcpPacket(pkt, ifindex, sip, dip, sport, dport, ack, hash_id, vrf_id, + ttl); uint8_t *ptr(new uint8_t[pkt->GetBuffLen()]); memcpy(ptr, pkt->GetBuff(), pkt->GetBuffLen()); client->agent_init()->pkt0()->ProcessFlowPacket(ptr, pkt->GetBuffLen(), diff --git a/src/vnsw/agent/pkt/test/test_pkt_util.h b/src/vnsw/agent/pkt/test/test_pkt_util.h index 1c0c45a38f4..f834092c9e3 100644 --- a/src/vnsw/agent/pkt/test/test_pkt_util.h +++ b/src/vnsw/agent/pkt/test/test_pkt_util.h @@ -31,7 +31,7 @@ extern void TxUdpPacket(int ifindex, const char *sip, const char *dip, extern void MakeTcpPacket(PktGen *pkt, int ifindex, const char *sip, const char *dip, uint16_t sport, uint16_t dport, - bool ack, int hash_id, uint32_t vrf_id); + bool ack, int hash_id, uint32_t vrf_id, int ttl = 0); extern void MakeSctpPacket(PktGen *pkt, int ifindex, const char *sip, const char *dip, uint16_t sport, uint16_t dport, @@ -39,7 +39,7 @@ extern void MakeSctpPacket(PktGen *pkt, int ifindex, const char *sip, extern void TxTcpPacket(int ifindex, const char *sip, const char *dip, uint16_t sport, uint16_t dport, bool ack, int hash_id = 1, - uint32_t vrf_id = -1); + uint32_t vrf_id = -1, int ttl = 0); extern void MakeIpMplsPacket(PktGen *pkt, int ifindex, const char *out_sip, const char *out_dip, uint32_t label, diff --git a/src/vnsw/agent/test/pkt_gen.h b/src/vnsw/agent/test/pkt_gen.h index 8177a234383..a0eb902f791 100644 --- a/src/vnsw/agent/test/pkt_gen.h +++ b/src/vnsw/agent/test/pkt_gen.h @@ -298,7 +298,7 @@ class PktGen { }; void AddIpHdr(const char *sip, const char *dip, uint16_t proto, - bool fragment = false) { + bool fragment = false, int ttl = 0) { struct ip *ip = (struct ip *)(buff + len); ip->ip_hl = 5; @@ -307,6 +307,7 @@ class PktGen { ip->ip_src.s_addr = inet_addr(sip); ip->ip_dst.s_addr = inet_addr(dip); ip->ip_p = proto; + ip->ip_ttl = ttl; if (fragment) ip->ip_off = htons(IP_MF); len += sizeof(struct ip); diff --git a/src/vnsw/agent/vrouter/ksync/flowtable_ksync.cc b/src/vnsw/agent/vrouter/ksync/flowtable_ksync.cc index dbcd2bea98f..337e2f5e859 100644 --- a/src/vnsw/agent/vrouter/ksync/flowtable_ksync.cc +++ b/src/vnsw/agent/vrouter/ksync/flowtable_ksync.cc @@ -327,6 +327,7 @@ int FlowTableKSyncEntry::Encode(sandesh_op::type op, char *buf, int buf_len) { flags |= VR_FLOW_FLAG_DPAT; } } + //TODO Seperate flags for BgpRouterService?? if (nat_flow->is_flags_set(FlowEntry::LinkLocalBindLocalSrcPort) || nat_flow->is_flags_set(FlowEntry::BgpRouterService)) { @@ -370,6 +371,7 @@ int FlowTableKSyncEntry::Encode(sandesh_op::type op, char *buf, int buf_len) { req.set_fr_action(action); req.set_fr_drop_reason(drop_reason); req.set_fr_qos_id(qos_config_idx); + req.set_fr_ttl(flow_entry_->data().ttl); } FlowProto *proto = ksync_obj_->ksync()->agent()->pkt()->get_flow_proto();