bgpd: align fragment flowspec decoding with tcpflags

As fragment bitmask and tcpflags bitmask in flowspec protocol is encoded
in the same way, it is not necessary to differentiate those two fields.
Moreover, it overrides the initial fragment limit set to 1. It is now
possible to handle multiple framgent values.

Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>

diff --git a/bgpd/bgp_flowspec.c b/bgpd/bgp_flowspec.c
index 5db7e37089bf49e7953b3c2c31536eb71e232d61..5f13e6a9dc2363c858b4a5e451c2ca4b325b2e5c 100644 (file)
--- a/bgpd/bgp_flowspec.c
+++ b/bgpd/bgp_flowspec.c
@@ -59,7 +59,8 @@ static int bgp_fs_nlri_validate(uint8_t *nlri_content, uint32_t len)
                                                   len - offset, NULL, &error);
                        break;
                case FLOWSPEC_TCP_FLAGS:
-                       ret = bgp_flowspec_tcpflags_decode(
+               case FLOWSPEC_FRAGMENT:
+                       ret = bgp_flowspec_bitmask_decode(
                                                   BGP_FLOWSPEC_VALIDATE_ONLY,
                                                   nlri_content + offset,
                                                   len - offset, NULL, &error);
@@ -71,12 +72,6 @@ static int bgp_fs_nlri_validate(uint8_t *nlri_content, uint32_t len)
                                                nlri_content + offset,
                                                len - offset, NULL, &error);
                        break;
-               case FLOWSPEC_FRAGMENT:
-                       ret = bgp_flowspec_fragment_type_decode(
-                                               BGP_FLOWSPEC_VALIDATE_ONLY,
-                                               nlri_content + offset,
-                                               len - offset, NULL, &error);
-                       break;
                default:
                        error = -1;
                        break;

diff --git a/bgpd/bgp_flowspec_util.c b/bgpd/bgp_flowspec_util.c
index db85c642aacb8827e9e6f0f45cbc798f9ff5d5cb..1b874276613c2a32150578c30a3a173761e2cbff 100644 (file)
--- a/bgpd/bgp_flowspec_util.c
+++ b/bgpd/bgp_flowspec_util.c
@@ -124,8 +124,9 @@ static bool bgp_flowspec_contains_prefix(struct prefix *pfs,
                                                     len - offset,
                                                     NULL, &error);
                        break;
+               case FLOWSPEC_FRAGMENT:
                case FLOWSPEC_TCP_FLAGS:
-                       ret = bgp_flowspec_tcpflags_decode(
+                       ret = bgp_flowspec_bitmask_decode(
                                                BGP_FLOWSPEC_VALIDATE_ONLY,
                                                nlri_content+offset,
                                                len - offset,
@@ -139,13 +140,6 @@ static bool bgp_flowspec_contains_prefix(struct prefix *pfs,
                                                len - offset, NULL,
                                                &error);
                        break;
-               case FLOWSPEC_FRAGMENT:
-                       ret = bgp_flowspec_fragment_type_decode(
-                                               BGP_FLOWSPEC_VALIDATE_ONLY,
-                                               nlri_content + offset,
-                                               len - offset, NULL,
-                                               &error);
-                       break;
                default:
                        error = -1;
                        break;
@@ -312,14 +306,14 @@ int bgp_flowspec_op_decode(enum bgp_flowspec_util_nlri_t type,
 
 
 /*
- * handle the flowspec tcpflags field
+ * handle the flowspec tcpflags or fragment field
  * return number of bytes analysed
  * if there is an error, the passed error param is used to give error:
  * -1 if decoding error,
  * if result is a string, its assumed length
  *  is BGP_FLOWSPEC_STRING_DISPLAY_MAX
  */
-int bgp_flowspec_tcpflags_decode(enum bgp_flowspec_util_nlri_t type,
+int bgp_flowspec_bitmask_decode(enum bgp_flowspec_util_nlri_t type,
                                 uint8_t *nlri_ptr,
                                 uint32_t max_len,
                                 void *result, int *error)
@@ -420,92 +414,6 @@ int bgp_flowspec_tcpflags_decode(enum bgp_flowspec_util_nlri_t type,
        return offset;
 }
 
-/*
- * handle the flowspec fragment type field
- * return error (returned values are invalid) or number of bytes analysed
- * -1 if error in decoding
- * >= 0 : number of bytes analysed (ok).
- */
-int bgp_flowspec_fragment_type_decode(enum bgp_flowspec_util_nlri_t type,
-                                     uint8_t *nlri_ptr,
-                                     uint32_t max_len,
-                                     void *result, int *error)
-{
-       int op[8];
-       int len, value, value_size, loop = 0;
-       char *ptr = (char *)result; /* for return_string */
-       struct bgp_pbr_fragment_val *mval =
-               (struct bgp_pbr_fragment_val *)result;
-       uint32_t offset = 0;
-       int len_string = BGP_FLOWSPEC_STRING_DISPLAY_MAX;
-       int len_written;
-
-       *error = 0;
-       do {
-               hex2bin(&nlri_ptr[offset], op);
-               offset++;
-               len = 2 * op[2] + op[3];
-               value_size = 1 << len;
-               value = hexstr2num(&nlri_ptr[offset], value_size);
-               if (value != 1 && value != 2 && value != 4 && value != 8)
-                       *error = -1;
-               offset += value_size;
-               /* TODO : as per RFC5574 : first Fragment bits are Reserved
-                * does that mean that it is not possible
-                * to handle multiple occurences ?
-                * as of today, we only grab the first TCP fragment
-                */
-               if (loop) {
-                       *error = -2;
-                       loop++;
-                       continue;
-               }
-               switch (type) {
-               case BGP_FLOWSPEC_RETURN_STRING:
-                       switch (value) {
-                       case 1:
-                               len_written = snprintf(ptr, len_string,
-                                                      "dont-fragment");
-                               len_string -= len_written;
-                               ptr += len_written;
-                               break;
-                       case 2:
-                               len_written = snprintf(ptr, len_string,
-                                                     "is-fragment");
-                               len_string -= len_written;
-                               ptr += len_written;
-                               break;
-                       case 4:
-                               len_written = snprintf(ptr, len_string,
-                                                      "first-fragment");
-                               len_string -= len_written;
-                               ptr += len_written;
-                               break;
-                       case 8:
-                               len_written = snprintf(ptr, len_string,
-                                                      "last-fragment");
-                               len_string -= len_written;
-                               ptr += len_written;
-                               break;
-                       default:
-                               {}
-                       }
-                       break;
-               case BGP_FLOWSPEC_CONVERT_TO_NON_OPAQUE:
-                       mval->bitmask = (uint8_t)value;
-                       break;
-               case BGP_FLOWSPEC_VALIDATE_ONLY:
-               default:
-                       /* no action */
-                       break;
-               }
-               loop++;
-       } while (op[0] == 0 && offset < max_len - 1);
-       if (offset > max_len)
-               *error = -1;
-       return offset;
-}
-
 int bgp_flowspec_match_rules_fill(uint8_t *nlri_content, int len,
                                  struct bgp_pbr_entry_main *bpem)
 {
@@ -624,7 +532,7 @@ int bgp_flowspec_match_rules_fill(uint8_t *nlri_content, int len,
                                                        &error);
                        break;
                case FLOWSPEC_TCP_FLAGS:
-                       ret = bgp_flowspec_tcpflags_decode(
+                       ret = bgp_flowspec_bitmask_decode(
                                        BGP_FLOWSPEC_CONVERT_TO_NON_OPAQUE,
                                        nlri_content + offset,
                                        len - offset,
@@ -638,7 +546,7 @@ int bgp_flowspec_match_rules_fill(uint8_t *nlri_content, int len,
                        offset += ret;
                        break;
                case FLOWSPEC_FRAGMENT:
-                       ret = bgp_flowspec_fragment_type_decode(
+                       ret = bgp_flowspec_bitmask_decode(
                                        BGP_FLOWSPEC_CONVERT_TO_NON_OPAQUE,
                                        nlri_content + offset,
                                        len - offset, &bpem->fragment,
@@ -647,7 +555,7 @@ int bgp_flowspec_match_rules_fill(uint8_t *nlri_content, int len,
                                zlog_err("%s: flowspec_fragment_type_decode error %d",
                                         __func__, error);
                        else
-                               bpem->match_bitmask |= FRAGMENT_PRESENT;
+                               bpem->match_fragment_num = error;
                        offset += ret;
                        break;
                default:

diff --git a/bgpd/bgp_flowspec_util.h b/bgpd/bgp_flowspec_util.h
index e4454ab4dba3fe87876ab40def2a74a8cf1abb98..2d16e57a36fdc9de6dd09fc2e5cb98797488687b 100644 (file)
--- a/bgpd/bgp_flowspec_util.h
+++ b/bgpd/bgp_flowspec_util.h
@@ -41,15 +41,11 @@ extern int bgp_flowspec_ip_address(enum bgp_flowspec_util_nlri_t type,
                                   uint32_t max_len,
                                   void *result, int *error);
 
-extern int bgp_flowspec_tcpflags_decode(enum bgp_flowspec_util_nlri_t type,
+extern int bgp_flowspec_bitmask_decode(enum bgp_flowspec_util_nlri_t type,
                                        uint8_t *nlri_ptr,
                                        uint32_t max_len,
                                        void *result, int *error);
 
-extern int bgp_flowspec_fragment_type_decode(enum bgp_flowspec_util_nlri_t type,
-                                            uint8_t *nlri_ptr,
-                                            uint32_t max_len,
-                                            void *result, int *error);
 struct bgp_pbr_entry_main;
 extern int bgp_flowspec_match_rules_fill(uint8_t *nlri_content, int len,
                                         struct bgp_pbr_entry_main *bpem);

diff --git a/bgpd/bgp_flowspec_vty.c b/bgpd/bgp_flowspec_vty.c
index 6b0390ed0fb3d5d5d8538a440beae8a8f6764bec..90acd8fcb16f48beb0f07ac2436dae0eacd5c4d2 100644 (file)
--- a/bgpd/bgp_flowspec_vty.c
+++ b/bgpd/bgp_flowspec_vty.c
@@ -173,7 +173,7 @@ void bgp_fs_nlri_get_string(unsigned char *nlri_content, size_t len,
                        ptr += len_written;
                        break;
                case FLOWSPEC_TCP_FLAGS:
-                       ret = bgp_flowspec_tcpflags_decode(
+                       ret = bgp_flowspec_bitmask_decode(
                                              type_util,
                                              nlri_content+offset,
                                              len - offset,
@@ -221,11 +221,11 @@ void bgp_fs_nlri_get_string(unsigned char *nlri_content, size_t len,
                        ptr += len_written;
                        break;
                case FLOWSPEC_FRAGMENT:
-                       ret = bgp_flowspec_fragment_type_decode(
-                                               type_util,
-                                               nlri_content + offset,
-                                               len - offset, local_string,
-                                               &error);
+                       ret = bgp_flowspec_bitmask_decode(
+                                             type_util,
+                                             nlri_content+offset,
+                                             len - offset,
+                                             local_string, &error);
                        if (ret <= 0)
                                break;
                        if (json_path) {

diff --git a/bgpd/bgp_pbr.c b/bgpd/bgp_pbr.c
index ac56369bf1cd4788e94bb7e8e28aaeb9186cae11..1fbc9826b2946c0aeb5fc1b0aa1f21b43a4c482d 100644 (file)
--- a/bgpd/bgp_pbr.c
+++ b/bgpd/bgp_pbr.c
@@ -1071,10 +1071,11 @@ void bgp_pbr_print_policy_route(struct bgp_pbr_entry_main *api)
                ptr += sprintf_bgp_pbr_match_val(ptr, &api->tcpflags[i],
                                         i > 0 ? NULL : "@tcpflags ");
 
-       if (api->match_bitmask & FRAGMENT_PRESENT) {
+       if (api->match_fragment_num)
                INCREMENT_DISPLAY(ptr, nb_items);
-               ptr += sprintf(ptr, "@fragment %u", api->fragment.bitmask);
-       }
+       for (i = 0; i < api->match_fragment_num; i++)
+               ptr += sprintf_bgp_pbr_match_val(ptr, &api->fragment[i],
+                                        i > 0 ? NULL : "@fragment ");
        if (!nb_items)
                ptr = return_string;
        else

diff --git a/bgpd/bgp_pbr.h b/bgpd/bgp_pbr.h
index 8a95b85df3e6f49d28f64d8e5f7d036aa36febef..d63d3c89c8f7125b9d9786bff8370c8216589c9c 100644 (file)
--- a/bgpd/bgp_pbr.h
+++ b/bgpd/bgp_pbr.h
@@ -107,7 +107,6 @@ struct bgp_pbr_entry_main {
 
 #define PREFIX_SRC_PRESENT (1 << 0)
 #define PREFIX_DST_PRESENT (1 << 1)
-#define FRAGMENT_PRESENT   (1 << 2)
        uint8_t match_bitmask;
 
        uint8_t match_src_port_num;
@@ -119,6 +118,7 @@ struct bgp_pbr_entry_main {
        uint8_t match_packet_length_num;
        uint8_t match_dscp_num;
        uint8_t match_tcpflags_num;
+       uint8_t match_fragment_num;
 
        struct prefix src_prefix;
        struct prefix dst_prefix;
@@ -136,7 +136,7 @@ struct bgp_pbr_entry_main {
        struct bgp_pbr_match_val dscp[BGP_PBR_MATCH_VAL_MAX];
 
        struct bgp_pbr_match_val tcpflags[BGP_PBR_MATCH_VAL_MAX];
-       struct bgp_pbr_fragment_val fragment;
+       struct bgp_pbr_match_val fragment[BGP_PBR_MATCH_VAL_MAX];
 
        uint16_t action_num;
        struct bgp_pbr_entry_action actions[ACTIONS_MAX_NUM];