flowprobe: fix tx flows generated for rewritten traffic

Currently, when IPFIX records generation is enabled for an interface in
the TX direction, some rewritten traffic is being sent from that
interface, and the Ethernet header's location has changed due to
rewriting, generated TX flows will contain fields with wrong and zero
values. For example, that can be observed when traffic is rewritten from
a subinterface to a hardware interface (i.e. when tags are removed). A
TX flow generated in this case will have wrong L2 fields because of an
incorrectly located Ethernet header. And zero L3/L4 fields because the
Ethernet type will match neither IP4 nor IP6.

The same code is executed to generate flows for both input and output
features. And the same mechanism is applied to identify the Ethernet
header in the buffer's data. However, such general code usually works
with the buffer's data conditionally based on the direction. For most
input features, the buffer's current_data will likely point to the IP
header. For most output features, the buffer's current_data will likely
point to the Ethernet header.

With this fix:
 - Keep relying on ethernet_buffer_get_header() to locate the Ethernet
   header for input features. And start using vlib_buffer_get_current()
   to locate the Ethernet header for output features. The function will
   account for the Ethernet header's position change in the buffer's
   data if there is rewriting.

 - After fixing Ethernet header determination in the buffer's data,
   L3/L4 fields will contain non-zero but still incorrect data. That is
   because IP header determination needs to be fixed too. It currently
   relies on the fact that the Ethernet header is always located at the
   beginning of the buffer's data and that l2_hdr_sz can be used as an
   IP header offset. However, this may not be the case after rewriting.
   So start calculating the actual offset of the IP header in the
   buffer's data.

 - Add a unit test to cover the case.

Type: fix
Change-Id: Icf3f9e6518912d06dff0d5aa48e103b3dc94edb7
Signed-off-by: Alexander Chernavin <achernavin@netgate.com>
This commit is contained in:
Alexander Chernavin
2023-11-07 11:25:21 +00:00
committed by Matthew Smith
parent 4aaedaa2ff
commit 64d6463d2e
2 changed files with 82 additions and 5 deletions

View File

@ -384,9 +384,11 @@ add_to_flow_record_state (vlib_main_t *vm, vlib_node_runtime_t *node,
flowprobe_record_t flags = fm->context[which].flags;
bool collect_ip4 = false, collect_ip6 = false;
ASSERT (b);
ethernet_header_t *eth = ethernet_buffer_get_header (b);
ethernet_header_t *eth = (direction == FLOW_DIRECTION_TX) ?
vlib_buffer_get_current (b) :
ethernet_buffer_get_header (b);
u16 ethertype = clib_net_to_host_u16 (eth->type);
u16 l2_hdr_sz = sizeof (ethernet_header_t);
u16 l3_hdr_offset = (u8 *) eth - b->data + sizeof (ethernet_header_t);
/* *INDENT-OFF* */
flowprobe_key_t k = {};
/* *INDENT-ON* */
@ -423,13 +425,13 @@ add_to_flow_record_state (vlib_main_t *vm, vlib_node_runtime_t *node,
while (clib_net_to_host_u16 (ethv->type) == ETHERNET_TYPE_VLAN)
{
ethv++;
l2_hdr_sz += sizeof (ethernet_vlan_header_tv_t);
l3_hdr_offset += sizeof (ethernet_vlan_header_tv_t);
}
k.ethertype = ethertype = clib_net_to_host_u16 ((ethv)->type);
}
if (collect_ip6 && ethertype == ETHERNET_TYPE_IP6)
{
ip6 = (ip6_header_t *) (b->data + l2_hdr_sz);
ip6 = (ip6_header_t *) (b->data + l3_hdr_offset);
if (flags & FLOW_RECORD_L3)
{
k.src_address.as_u64[0] = ip6->src_address.as_u64[0];
@ -448,7 +450,7 @@ add_to_flow_record_state (vlib_main_t *vm, vlib_node_runtime_t *node,
}
if (collect_ip4 && ethertype == ETHERNET_TYPE_IP4)
{
ip4 = (ip4_header_t *) (b->data + l2_hdr_sz);
ip4 = (ip4_header_t *) (b->data + l3_hdr_offset);
if (flags & FLOW_RECORD_L3)
{
k.src_address.ip4.as_u32 = ip4->src_address.as_u32;

View File

@ -30,6 +30,7 @@ from vpp_ip_route import VppIpRoute, VppRoutePath
from vpp_papi.macaddress import mac_ntop
from socket import inet_ntop
from vpp_papi import VppEnum
from vpp_sub_interface import VppDot1ADSubint
TMPL_COMMON_FIELD_COUNT = 6
@ -40,6 +41,9 @@ TMPL_L4_FIELD_COUNT = 3
IPFIX_TCP_FLAGS_ID = 6
IPFIX_SRC_TRANS_PORT_ID = 7
IPFIX_DST_TRANS_PORT_ID = 11
IPFIX_SRC_IP4_ADDR_ID = 8
IPFIX_DST_IP4_ADDR_ID = 12
IPFIX_FLOW_DIRECTION_ID = 61
TCP_F_FIN = 0x01
TCP_F_SYN = 0x02
@ -1230,6 +1234,77 @@ class DatapathTx(MethodHolder, DatapathTestsHolder):
intf3 = "pg6"
direction = "tx"
def test_rewritten_traffic(self):
"""Rewritten traffic (from subif to ipfix if)"""
self.pg_enable_capture(self.pg_interfaces)
self.pkts = []
# prepare a sub-interface
subif = VppDot1ADSubint(self, self.pg7, 0, 300, 400)
subif.admin_up()
subif.config_ip4()
# enable ip4 datapath for an interface
ipfix = VppCFLOW(
test=self,
intf="pg8",
datapath="ip4",
layer="l2 l3 l4",
direction=self.direction,
)
ipfix.add_vpp_config()
# template packet should arrive immediately
ipfix_decoder = IPFIXDecoder()
templates = ipfix.verify_templates(ipfix_decoder, count=1)
# forward some traffic through the ipfix interface
route = VppIpRoute(
self,
"9.0.0.0",
24,
[VppRoutePath(self.pg8.remote_ip4, self.pg8.sw_if_index)],
)
route.add_vpp_config()
# prepare an IPv4 packet (subif => ipfix interface)
pkt = (
Ether(src=subif.remote_mac, dst=self.pg7.local_mac)
/ IP(src=subif.remote_ip4, dst="9.0.0.1")
/ UDP(sport=1234, dport=4321)
/ Raw(b"\xa5" * 123)
)
self.pkts = [
subif.add_dot1ad_layer(pkt, 300, 400),
]
# send the packet
capture = self.send_packets(self.pg7, self.pg8)
# wait for a flow and verify it
self.vapi.ipfix_flush()
cflow = self.wait_for_cflow_packet(self.collector, templates[0])
self.verify_cflow_data(ipfix_decoder, capture, cflow)
self.verify_cflow_data_detail(
ipfix_decoder,
capture,
cflow,
{
IPFIX_SRC_IP4_ADDR_ID: "src_ip",
IPFIX_DST_IP4_ADDR_ID: "dst_ip",
IPFIX_SRC_TRANS_PORT_ID: "sport",
IPFIX_DST_TRANS_PORT_ID: "dport",
IPFIX_FLOW_DIRECTION_ID: (self.direction == "tx"),
},
)
self.collector.get_capture(2)
# cleanup
route.remove_vpp_config()
subif.remove_vpp_config()
ipfix.remove_vpp_config()
@tag_fixme_vpp_workers
class DatapathRx(MethodHolder, DatapathTestsHolder):