From 4189108e1dbac0a7961d31d59a21b7fe8354ce61 Mon Sep 17 00:00:00 2001 From: Filip Varga Date: Mon, 30 Aug 2021 16:23:38 +0200 Subject: [PATCH] nat: NAT44 ED api fix and improvement Backward compatibility fix returns erroneous behavior that lets user add internally unused inside interface for the purpose of complying with the old add/dump/details API behavior. Change introduced in https://gerrit.fd.io/r/c/vpp/+/32951 removed extra inside interface that wasn't required or any how used by the output feature. This patch also changed outside interface flags to inside & outside. This fix returns the old behavior by imitating the old behavior through dummy registratoin data. Added new API calls nat44_ed_add_del_output_interface and nat44_ed_output_interface_get/details as a replacement of old API's. New API introduces simplified and cleaner way of configuring outside feature without requirement of config flags. Type: improvement Signed-off-by: Filip Varga Change-Id: I7a170f7325727c04da5e2e3ffbe3f02179531284 --- src/plugins/nat/nat44-ed/nat44_ed.api | 42 +++++++++ src/plugins/nat/nat44-ed/nat44_ed.h | 2 + src/plugins/nat/nat44-ed/nat44_ed_api.c | 148 +++++++++++++++++++++++++++++--- 3 files changed, 178 insertions(+), 14 deletions(-) diff --git a/src/plugins/nat/nat44-ed/nat44_ed.api b/src/plugins/nat/nat44-ed/nat44_ed.api index bff00b809e0..36637b26246 100644 --- a/src/plugins/nat/nat44-ed/nat44_ed.api +++ b/src/plugins/nat/nat44-ed/nat44_ed.api @@ -729,6 +729,7 @@ define nat44_interface_details { @param sw_if_index - software index of the interface */ autoreply define nat44_interface_add_del_output_feature { + option deprecated; u32 client_index; u32 context; bool is_add; @@ -741,6 +742,7 @@ autoreply define nat44_interface_add_del_output_feature { @param context - sender context, to match reply w/ request */ define nat44_interface_output_feature_dump { + option deprecated; u32 client_index; u32 context; }; @@ -752,11 +754,51 @@ define nat44_interface_output_feature_dump { @param sw_if_index - software index of the interface */ define nat44_interface_output_feature_details { + option deprecated; u32 context; vl_api_nat_config_flags_t flags; vl_api_interface_index_t sw_if_index; }; +/** \brief add/del NAT output interface (postrouting + in2out translation) + @param client_index - opaque cookie to identify the sender + @param context - sender context, to match reply w/ request + @param is_add - true if add, false if delete + @param sw_if_index - software index of the interface +*/ +autoendian autoreply define nat44_ed_add_del_output_interface { + u32 client_index; + u32 context; + bool is_add; + vl_api_interface_index_t sw_if_index; +}; + +service { + rpc nat44_ed_output_interface_get returns nat44_ed_output_interface_get_reply + stream nat44_ed_output_interface_details; +}; + +define nat44_ed_output_interface_get +{ + u32 client_index; + u32 context; + u32 cursor; +}; + +define nat44_ed_output_interface_get_reply +{ + u32 context; + i32 retval; + u32 cursor; +}; + +define nat44_ed_output_interface_details +{ + u32 context; + vl_api_interface_index_t sw_if_index; +}; + /** \brief Add/delete NAT44 static mapping @param client_index - opaque cookie to identify the sender @param context - sender context, to match reply w/ request diff --git a/src/plugins/nat/nat44-ed/nat44_ed.h b/src/plugins/nat/nat44-ed/nat44_ed.h index f8cd9a06bea..e2f5810f9b0 100644 --- a/src/plugins/nat/nat44-ed/nat44_ed.h +++ b/src/plugins/nat/nat44-ed/nat44_ed.h @@ -540,6 +540,8 @@ typedef struct snat_main_s /* Interface pool */ snat_interface_t *interfaces; snat_interface_t *output_feature_interfaces; + // broken api backward compatibility + snat_interface_t *output_feature_dummy_interfaces; /* Vector of outside addresses */ snat_address_t *addresses; diff --git a/src/plugins/nat/nat44-ed/nat44_ed_api.c b/src/plugins/nat/nat44-ed/nat44_ed_api.c index d4de4823025..ad00d11052b 100644 --- a/src/plugins/nat/nat44-ed/nat44_ed_api.c +++ b/src/plugins/nat/nat44-ed/nat44_ed_api.c @@ -426,9 +426,50 @@ vl_api_nat44_interface_dump_t_handler (vl_api_nat44_interface_dump_t * mp) return; pool_foreach (i, sm->interfaces) - { - send_nat44_interface_details(i, reg, mp->context); - } + { + send_nat44_interface_details (i, reg, mp->context); + } +} + +static_always_inline int +add_del_dummy_output_interface (u32 sw_if_index, u8 is_inside, u8 is_add) +{ + snat_main_t *sm = &snat_main; + snat_interface_t *i; + int rv = 1; + + pool_foreach (i, sm->output_feature_dummy_interfaces) + { + if (i->sw_if_index == sw_if_index) + { + if (!is_add) + { + pool_put (sm->output_feature_dummy_interfaces, i); + rv = 0; + } + goto done; + } + } + + if (is_add) + { + pool_get (sm->output_feature_dummy_interfaces, i); + i->sw_if_index = sw_if_index; + + if (is_inside) + { + i->flags |= NAT_INTERFACE_FLAG_IS_INSIDE; + } + else + { + i->flags |= NAT_INTERFACE_FLAG_IS_OUTSIDE; + } + + rv = 0; + } + +done: + return rv; } static void @@ -444,13 +485,20 @@ static void sw_if_index = ntohl (mp->sw_if_index); - if (mp->is_add) - { - rv = nat44_ed_add_output_interface (sw_if_index); - } - else + // register all interfaces in the dummy structure + rv = add_del_dummy_output_interface ( + sw_if_index, mp->flags & NAT_API_IS_INSIDE, mp->is_add); + + if (!(mp->flags & NAT_API_IS_INSIDE)) { - rv = nat44_ed_del_output_interface (sw_if_index); + if (mp->is_add) + { + rv = nat44_ed_add_output_interface (sw_if_index); + } + else + { + rv = nat44_ed_del_output_interface (sw_if_index); + } } BAD_SW_IF_INDEX_LABEL; @@ -473,7 +521,9 @@ send_nat44_interface_output_feature_details (snat_interface_t * i, rmp->context = context; if (nat44_ed_is_interface_inside (i)) - rmp->flags |= NAT_API_IS_INSIDE; + { + rmp->flags |= NAT_API_IS_INSIDE; + } vl_api_send_msg (reg, (u8 *) rmp); } @@ -490,10 +540,80 @@ static void if (!reg) return; - pool_foreach (i, sm->output_feature_interfaces) - { - send_nat44_interface_output_feature_details (i, reg, mp->context); - } + pool_foreach (i, sm->output_feature_dummy_interfaces) + { + send_nat44_interface_output_feature_details (i, reg, mp->context); + } +} + +static void +vl_api_nat44_ed_add_del_output_interface_t_handler ( + vl_api_nat44_ed_add_del_output_interface_t *mp) +{ + vl_api_nat44_ed_add_del_output_interface_reply_t *rmp; + snat_main_t *sm = &snat_main; + u32 sw_if_index; + int rv = 0; + + VALIDATE_SW_IF_INDEX (mp); + + sw_if_index = ntohl (mp->sw_if_index); + + if (mp->is_add) + { + rv = nat44_ed_add_output_interface (sw_if_index); + } + else + { + rv = nat44_ed_del_output_interface (sw_if_index); + } + + BAD_SW_IF_INDEX_LABEL; + REPLY_MACRO (VL_API_NAT44_ED_ADD_DEL_OUTPUT_INTERFACE_REPLY); +} + +#define vl_endianfun +#include +#undef vl_endianfun +static void +send_nat44_ed_output_interface_details (u32 index, vl_api_registration_t *rp, + u32 context) +{ + snat_main_t *sm = &snat_main; + vl_api_nat44_ed_output_interface_details_t *rmp; + snat_interface_t *i = + pool_elt_at_index (sm->output_feature_interfaces, index); + + /* Make sure every field is initiated (or don't skip the clib_memset()) */ + REPLY_MACRO_DETAILS4 ( + VL_API_NAT44_ED_OUTPUT_INTERFACE_DETAILS, rp, context, ({ + rmp->sw_if_index = i->sw_if_index; + + /* Endian hack until apigen registers _details + * endian functions */ + vl_api_nat44_ed_output_interface_details_t_endian (rmp); + rmp->_vl_msg_id = htons (rmp->_vl_msg_id); + rmp->context = htonl (rmp->context); + })); +} + +static void +vl_api_nat44_ed_output_interface_get_t_handler ( + vl_api_nat44_ed_output_interface_get_t *mp) +{ + vl_api_nat44_ed_output_interface_get_reply_t *rmp; + snat_main_t *sm = &snat_main; + i32 rv = 0; + + if (pool_elts (sm->output_feature_interfaces) == 0) + { + REPLY_MACRO (VL_API_NAT44_ED_OUTPUT_INTERFACE_GET_REPLY); + return; + } + + REPLY_AND_DETAILS_MACRO ( + VL_API_NAT44_ED_OUTPUT_INTERFACE_GET_REPLY, sm->output_feature_interfaces, + ({ send_nat44_ed_output_interface_details (cursor, rp, mp->context); })); } static void -- 2.16.6