From 95c2da7c251a87a4a9e8c618d76df0502e7b24f4 Mon Sep 17 00:00:00 2001 From: Nobuhiro MIKI Date: Wed, 28 Jun 2023 15:15:58 +0900 Subject: lb: Fix src_ip_sticky evaluation bug in per-port-vip case. Before this fix, the src_ip_sticky flag was passed as an argument to the lb_node_get_hash function, which computes a hash value for a packet. However, in per-port-vip case, the value of src_ip_sticky flag may be different for each port number. As a result, the value is the same for all port numbers, even though it is a per-port-vip case. This commit fixes the src_ip_sticky evaluation by delaying it until the packet is received, so that the correct value is obtained. Also, the unit test case has been enhanced for this bug fix. The steps to reproduce this bug are described below: https://lists.fd.io/g/vpp-dev/message/23248 Type: fix Fixes: 613e6dc0bf92 ("lb: add source ip based sticky load balancing") Change-Id: I483492b214a1768e7a21fd86edd5151b3c46528b Signed-off-by: Nobuhiro MIKI --- src/plugins/lb/lb.h | 77 ++++++----------------------------------------------- 1 file changed, 8 insertions(+), 69 deletions(-) (limited to 'src/plugins/lb/lb.h') diff --git a/src/plugins/lb/lb.h b/src/plugins/lb/lb.h index fa1cfaadc25..46da40970c9 100644 --- a/src/plugins/lb/lb.h +++ b/src/plugins/lb/lb.h @@ -353,94 +353,41 @@ typedef struct { /* clang-format off */ #define lb_vip_is_gre4(vip) (((vip)->type == LB_VIP_TYPE_IP6_GRE4 \ || (vip)->type == LB_VIP_TYPE_IP4_GRE4) \ - && ((vip)->port == 0) \ - && !lb_vip_is_src_ip_sticky (vip)) + && ((vip)->port == 0)) #define lb_vip_is_gre6(vip) (((vip)->type == LB_VIP_TYPE_IP6_GRE6 \ || (vip)->type == LB_VIP_TYPE_IP4_GRE6) \ - && ((vip)->port == 0) \ - && !lb_vip_is_src_ip_sticky (vip)) + && ((vip)->port == 0)) #define lb_vip_is_gre4_port(vip) (((vip)->type == LB_VIP_TYPE_IP6_GRE4 \ || (vip)->type == LB_VIP_TYPE_IP4_GRE4) \ - && ((vip)->port != 0) \ - && !lb_vip_is_src_ip_sticky (vip)) + && ((vip)->port != 0)) #define lb_vip_is_gre6_port(vip) (((vip)->type == LB_VIP_TYPE_IP6_GRE6 \ || (vip)->type == LB_VIP_TYPE_IP4_GRE6) \ - && ((vip)->port != 0) \ - && !lb_vip_is_src_ip_sticky (vip)) + && ((vip)->port != 0)) /* clang-format on */ -#define lb_vip_is_gre4_sticky(vip) \ - (((vip)->type == LB_VIP_TYPE_IP6_GRE4 || \ - (vip)->type == LB_VIP_TYPE_IP4_GRE4) && \ - ((vip)->port == 0) && lb_vip_is_src_ip_sticky (vip)) - -#define lb_vip_is_gre6_sticky(vip) \ - (((vip)->type == LB_VIP_TYPE_IP6_GRE6 || \ - (vip)->type == LB_VIP_TYPE_IP4_GRE6) && \ - ((vip)->port == 0) && lb_vip_is_src_ip_sticky (vip)) - -#define lb_vip_is_gre4_port_sticky(vip) \ - (((vip)->type == LB_VIP_TYPE_IP6_GRE4 || \ - (vip)->type == LB_VIP_TYPE_IP4_GRE4) && \ - ((vip)->port != 0) && lb_vip_is_src_ip_sticky (vip)) - -#define lb_vip_is_gre6_port_sticky(vip) \ - (((vip)->type == LB_VIP_TYPE_IP6_GRE6 || \ - (vip)->type == LB_VIP_TYPE_IP4_GRE6) && \ - ((vip)->port != 0) && lb_vip_is_src_ip_sticky (vip)) - always_inline bool lb_vip_is_l3dsr(const lb_vip_t *vip) { - return (vip->type == LB_VIP_TYPE_IP4_L3DSR && vip->port == 0 && - !lb_vip_is_src_ip_sticky (vip)); + return (vip->type == LB_VIP_TYPE_IP4_L3DSR && vip->port == 0); } always_inline bool lb_vip_is_l3dsr_port(const lb_vip_t *vip) { - return (vip->type == LB_VIP_TYPE_IP4_L3DSR && vip->port != 0 && - !lb_vip_is_src_ip_sticky (vip)); + return (vip->type == LB_VIP_TYPE_IP4_L3DSR && vip->port != 0); } always_inline bool lb_vip_is_nat4_port(const lb_vip_t *vip) { - return (vip->type == LB_VIP_TYPE_IP4_NAT4 && vip->port != 0 && - !lb_vip_is_src_ip_sticky (vip)); + return (vip->type == LB_VIP_TYPE_IP4_NAT4 && vip->port != 0); } always_inline bool lb_vip_is_nat6_port(const lb_vip_t *vip) { - return (vip->type == LB_VIP_TYPE_IP6_NAT6 && vip->port != 0 && - !lb_vip_is_src_ip_sticky (vip)); -} - -always_inline bool -lb_vip_is_l3dsr_sticky (const lb_vip_t *vip) -{ - return (vip->type == LB_VIP_TYPE_IP4_L3DSR && vip->port == 0 && - lb_vip_is_src_ip_sticky (vip)); -} -always_inline bool -lb_vip_is_l3dsr_port_sticky (const lb_vip_t *vip) -{ - return (vip->type == LB_VIP_TYPE_IP4_L3DSR && vip->port != 0 && - lb_vip_is_src_ip_sticky (vip)); -} -always_inline bool -lb_vip_is_nat4_port_sticky (const lb_vip_t *vip) -{ - return (vip->type == LB_VIP_TYPE_IP4_NAT4 && vip->port != 0 && - lb_vip_is_src_ip_sticky (vip)); -} -always_inline bool -lb_vip_is_nat6_port_sticky (const lb_vip_t *vip) -{ - return (vip->type == LB_VIP_TYPE_IP6_NAT6 && vip->port != 0 && - lb_vip_is_src_ip_sticky (vip)); + return (vip->type == LB_VIP_TYPE_IP6_NAT6 && vip->port != 0); } format_function_t format_lb_vip; @@ -600,14 +547,6 @@ typedef struct { dpo_type_t dpo_l3dsr_port_type; dpo_type_t dpo_nat4_port_type; dpo_type_t dpo_nat6_port_type; - dpo_type_t dpo_gre4_sticky_type; - dpo_type_t dpo_gre6_sticky_type; - dpo_type_t dpo_gre4_port_sticky_type; - dpo_type_t dpo_gre6_port_sticky_type; - dpo_type_t dpo_l3dsr_sticky_type; - dpo_type_t dpo_l3dsr_port_sticky_type; - dpo_type_t dpo_nat4_port_sticky_type; - dpo_type_t dpo_nat6_port_sticky_type; /** * Node type for registering to fib changes. */ -- cgit 1.2.3-korg