OSPF6: Refactor to prepare for E-LSA handling #16533
Conversation
ghost
force-pushed
the
less-controversial-ospf6d-refactor-before-adding-tlvs
branch
from
9af3873 to
93cd6b9
Compare
|
ci:rerun |
| prefixnum = ntohl(link_lsa->prefix_num); | ||
| if (pos > prefixnum) | ||
| return NULL; | ||
| memcpy(&in6, OSPF6_PREFIX_BODY(prefix), |
There was a problem hiding this comment.
Should we consider a function to do this copy?
There was a problem hiding this comment.
Sorry, I'm not sure I fully understand the intent. Are you suggesting a function wrapping memcpy, in order to add explicit types, to improve the issue of potentially providing an incorrect addresses or length? It could be a number of different one-line function variations for different source and destination types. I will try it and see what it looks like.
There was a problem hiding this comment.
I just see us using the same logic in two places ... and I can where we might want to use this other places and/or provide a check at some point ... ?? Not a blocker, just asking if we should consider it.
There was a problem hiding this comment.
I can see what you mean now. There is such a function, called ospf6_prefix_in6_addr, so it is definitely an opportunity to improve consistency.
The implementation of that function seemed a bit unintuitive to me, and I see it was added in commit b8ce0c3 to address an OOB read flagged by Coverity, so I should take the time to understand it better.
| prefixnum = ntohs(intra_prefix_lsa->prefix_num); | ||
| if ((pos + 1) > prefixnum) | ||
| return NULL; | ||
| memcpy(&in6, OSPF6_PREFIX_BODY(prefix), |
|
Lints need to be taken care of. Failure is in nexthop groups, so they probably aren't related (?). |
| [0x00ff & OSPF6_LSTYPE_GRACE_LSA] = 0 | ||
| }; | ||
|
|
||
| void *lsdesc_start_lsa_type(struct ospf6_lsa_header *header, int lsa_type) |
There was a problem hiding this comment.
It is confusing to use "lsdesc_" for LSA types other than router-LSAs and network-LSAs.
| return (char *)lsa_after_header(header) + ospf6_lsa_min_size[t]; | ||
| } | ||
|
|
||
| void *lsdesc_start(struct ospf6_lsa_header *header) |
There was a problem hiding this comment.
Same comment about using "lsdesc_" for LSAs not using links.
ghost
force-pushed
the
less-controversial-ospf6d-refactor-before-adding-tlvs
branch
from
93cd6b9 to
a0200e8
Compare
|
@acooks-at-bda can you clear up the lint suggestions? thanks! |
ghost
force-pushed
the
less-controversial-ospf6d-refactor-before-adding-tlvs
branch
from
3373229 to
f969640
Compare
In preperation for Extended LSA types and their TLVs, factor out the TLV handling from the Gracefull Restart functionality. Signed-off-by: Andrew Cooks <[email protected]>
It will be cleaner to have the LSAs in a single header and the future TLVs in a single header. Signed-off-by: Andrew Cooks <[email protected]>
For readability Signed-off-by: Andrew Cooks <[email protected]>
Signed-off-by: Andrew Cooks <[email protected]>
The void * return type of the replacement enables the removal of a cast at every point of use, and the name no longer suggests that it points to the last byte of the header. Signed-off-by: Andrew Cooks <[email protected]>
Replaces open type casting and pointer arithmetic for readability. Signed-off-by: Andrew Cooks <[email protected]>
The original TLV_HDR_TOP implementation only worked for Graceful Restart LSAs, because they had no "LSA body". This change introduces a body size lookup table and changes the macro to a function that accounts for the LSA body for all LSA types, and provides type checking on the provided pointer before arithmetic. It also removes the open type casting and pointer arithmetic. The introduced lsdesc_start() is used to find the start of a descriptor, and will be used for TLVs in E-LSAs as well as old LSA. Signed-off-by: Andrew Cooks <[email protected]>
Add utility function to find the Nth router lsdesc or network lsdesc in an LSA. Signed-off-by: Andrew Cooks <[email protected]>
Signed-off-by: Andrew Cooks <[email protected]>
Improves code readability by reducing pointer casting and arithmetic, and intendation. Signed-off-by: Andrew Cooks <[email protected]>
Add utility function to find the Nth prefix in a link LSA or Intra Prefix LSA. Signed-off-by: Andrew Cooks <[email protected]>
Signed-off-by: Andrew Cooks <[email protected]>
Signed-off-by: Andrew Cooks <[email protected]>
Apply formatting changes suggested by CI frrbot. Signed-off-by: Andrew Cooks <[email protected]>
ghost
force-pushed
the
less-controversial-ospf6d-refactor-before-adding-tlvs
branch
from
f969640 to
b2526dd
Compare
This PR contains what I hope would be the less controversial bits of the Work In Progress on PR #16532.
It refactors the ospf6d implementation in order to