FRRouting / frr

The FRRouting Protocol Suite
https://frrouting.org/
Other
3.18k stars 1.23k forks source link

`show ip ospf database opaque-area ....` returns Wrong Adjacency SID TLV size on dump #14959

Closed donaldsharp closed 8 months ago

donaldsharp commented 8 months ago

When dumping the Adjacency Sid data, the ospf vty code expects it be 8 bytes of data but the value is 7.

      Opaque-Type 8 (Extended Link Opaque LSA)
      Opaque-ID   0x1
      Opaque-Info: 48 octets of data
      Extended Link TLV: Length 44
        Link Type: 0x1
        Link ID: 3.3.3.3
        Link data: 10.0.4.5
      Wrong Adjacency SID TLV size: 7(8). Abort!
      Wrong Adjacency SID TLV size: 7(8). Abort!
      Remote Interface Address Sub-TLV: Length 4
        Address: 10.0.4.3

Given that 7 is ok and not generating problems elsewhere. It sure seems like this is a problem in the dump code? When I modify the call to:

@@ -1735,7 +1735,7 @@ static uint16_t show_vty_ext_link_adj_sid(struct vty *vty,
 {
        struct ext_subtlv_adj_sid *top = (struct ext_subtlv_adj_sid *)tlvh;

-       check_tlv_size(EXT_SUBTLV_ADJ_SID_SIZE, "Adjacency SID");
+       check_tlv_size(EXT_SUBTLV_ADJ_SID_SIZE -1, "Adjacency SID");

        vty_out(vty,
                "  Adj-SID Sub-TLV: Length %u\n\tFlags: 0x%x\n\tMT-ID:0x%x\n\tWeight: 0x%x\n\t%s: %u\n",

The code dumps more data. Clearly either FRR is generating the lsa tlv incorrectly ( it should be 8 bytes ) and we are not inter-operable with other vendors or the EXT_SUBTLV_ADJ_SID_SIZE should be modified. Either way I don't understand enough about the rfc and code to actually fix this issue without spending more time.

I can make this happen reliably running the ospf_sr_te_topo1 and adding a dump command on rtr1 when check_bsid is called.

odd22 commented 8 months ago

Hi Donald,

The problem comes from the fact that SID could be an index or an mpls label. In the first case, the size is 4 bytes while in the second case the size is 3 bytes. So, depending if the router is advertising an index within the SRGB range for Prefix SID or within SRLB range for Adjacency SID, or advertising an MPLS label, the size may vary. So, the code should look into the SID type (index or MPLS label) to check the size. I'll propose a PR for that.

odd22 commented 8 months ago

Just published PR #14962 to correct this issue

huangyl-git commented 8 months ago

Nice! I had the same problem today, and just Find out what the problem is and you've solved it~~~