Skip to content

Fix done for nexthop as IPv4 mapped IPv6 address.#2

Closed
hariosniral wants to merge 1 commit into
danos:initialfrom
Niral-Networks:niral_6vpe_6pe
Closed

Fix done for nexthop as IPv4 mapped IPv6 address.#2
hariosniral wants to merge 1 commit into
danos:initialfrom
Niral-Networks:niral_6vpe_6pe

Conversation

@hariosniral
Copy link
Copy Markdown
Contributor

@hariosniral hariosniral commented May 25, 2020

  1. "RTF_MAPPED_IPV6" flag is set for IPv4 mapped IPv6 nexthop address.
  2. This changes are tested with danos-1908 revision.

@hariosniral
Fix done for nexthop as IPv4 mapped IPv6 address.
f3266c4 
1. "RTF_MAPPED_IPV6" flag is set for IPv4 mapped IPv6 nexthop address.

Signed-off-by: harios <hari@niralnetworks.com>
ghost
ghost reviewed Jun 10, 2020
View reviewed changes
Copy link
Copy Markdown

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your submission.

First of all, DANOS PRs should always be made against the head of development which for most repos (including vyatta-dataplane) is master.

Second of all, please refer to CONTRIBUTING.md about the coding style. In general the style should be consistent with the surrounding code. In particular, tabs should be used for indent rather than spaces and opening and closing block brackets should only be used for multi-line bodies of the block.

ghost
ghost reviewed Jun 10, 2020
View reviewed changes
Comment thread src/netinet6/route_v6.c
if (!(nl_flags & NL_FLAG_ANY_ADDR))
flags |= RTF_GATEWAY;

if (IN6_IS_ADDR_V4MAPPED(&gw)) {
Copy link
Copy Markdown

@ghost ghost Jun 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing the check only here won't the case of having an ECMP route, such as in the case of having an eiBGP 6PE/6VPE route. In addition, it possibly is confusing to the reader to look gw here, when it may or may not be used. Thus I would suggest pushing this check inside mpath conditional else block just below, and adding a similar check to the appropriate place in the logic called from ecmp6_create.

Copy link
Copy Markdown

@KaushikNiral KaushikNiral Jun 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Robert,

Thanks for your review.
We understood your corrections suggested.
In ecmp6_create() we see nexthop6_fill_mpls() is already having the similar check, so we are adding the check in nexthop6_fill().

Thanks,

@KaushikNiral
Copy link
Copy Markdown

KaushikNiral commented Jun 16, 2020

Thank you for your submission.

First of all, DANOS PRs should always be made against the head of development which for most repos (including vyatta-dataplane) is master.

Second of all, please refer to CONTRIBUTING.md about the coding style. In general the style should be consistent with the surrounding code. In particular, tabs should be used for indent rather than spaces and opening and closing block brackets should only be used for multi-line bodies of the block.

Thanks for your encouragement.
We will follow it and send PR with master branch.

@KaushikNiral
Copy link
Copy Markdown

KaushikNiral commented Jun 23, 2020

Hi Robert,

We have sent a new PR request against the master branch.
The new PR link is :- #3

Please provide your feedback on it.

Thanks,
Kaushik

@nickbroon
Copy link
Copy Markdown
Contributor

nickbroon commented Aug 3, 2020

Closed in favour of #3

@nickbroon nickbroon closed this Aug 3, 2020
VyattaInternal pushed a commit that referenced this pull request Feb 22, 2021
@srinivasknarayan
ut: Add test sip_nat20 to dp_test_npf_alg_sip_nat.c
8baf099 
A preliminary 'baseline' test of SIP data set #2 with no NAT configured.
VRVDR-54421
VyattaInternal pushed a commit that referenced this pull request Feb 22, 2021
@srinivasknarayan
ut: Add test sip_nat21 to dp_test_npf_alg_sip_nat.c
9e63ef7 
Tests SIP data set #2 with SNAT configured.
VRVDR-54421
VyattaInternal pushed a commit that referenced this pull request Mar 2, 2021
npf: use rcu safe null check for sentry
5605da3 
se_sen field in struct session may be NULL in two cases for newly
created sessions not yet added to sentry hash list, or after sesssion
removed from sentry hash list during reclaim in session gc.

This causes the following segmentaion fault infrequently.

[Current thread is 1 (Thread 0x7ff5c3fff700 (LWP 30235))]
 #0  0x000055ce31a8978d in csync_get_session_from_init_sentry (
    cse=<synthetic pointer>, cs=<synthetic pointer>, sp=0x7ff5f004ef36)
    at ../src/npf/csync/csync_session_unpack.c:38
 #1  csync_session_unpack_update (csu=0x7ff5f004ef2e)
    at ../src/npf/csync/csync_session_unpack.c:71
 #2  csync_unpack_session (size=<optimized out>, msg=0x7ff5f004ef26)
    at ../src/npf/csync/csync_session_unpack.c:421
 #3  csync_recv_session_update (frame=<optimized out>)
    at ../src/npf/csync/csync_session_unpack.c:501
 #4  0x000055ce31b2d57d in csync_restore_sessions (n=<optimized out>,
    flist=<optimized out>) at ../src/csync/csync_transfer.c:218
 #5  csync_pull_batch (info=0x7ff58400b880) at
../src/csync/csync_transfer.c:506
 #6  csync_xfer_backup (pipe=0x7ff58400b8e0, arg=0x7ff58400b880)
    at ../src/csync/csync_transfer.c:301
 #7  0x00007ff6629618d3 in ?? () from
/usr/lib/x86_64-linux-gnu/libczmq.so.4
 #8  0x00007ff6611ad4a4 in start_thread ()
   from /lib/x86_64-linux-gnu/libpthread.so.0
 #9  0x00007ff660eefd0f in clone () from /lib/x86_64-linux-gnu/libc.so.6

Fixed by doing a safe derefernce and checking for NULL.

VRVDR-54586
VyattaInternal pushed a commit that referenced this pull request Mar 3, 2021
npf: use rcu safe null check for sentry
81f5f2d 
se_sen field in struct session may be NULL in two cases for newly
created sessions not yet added to sentry hash list, or after sesssion
removed from sentry hash list during reclaim in session gc.

This causes the following segmentaion fault infrequently.

[Current thread is 1 (Thread 0x7ff5c3fff700 (LWP 30235))]
 #0  0x000055ce31a8978d in csync_get_session_from_init_sentry (
    cse=<synthetic pointer>, cs=<synthetic pointer>, sp=0x7ff5f004ef36)
    at ../src/npf/csync/csync_session_unpack.c:38
 #1  csync_session_unpack_update (csu=0x7ff5f004ef2e)
    at ../src/npf/csync/csync_session_unpack.c:71
 #2  csync_unpack_session (size=<optimized out>, msg=0x7ff5f004ef26)
    at ../src/npf/csync/csync_session_unpack.c:421
 #3  csync_recv_session_update (frame=<optimized out>)
    at ../src/npf/csync/csync_session_unpack.c:501
 #4  0x000055ce31b2d57d in csync_restore_sessions (n=<optimized out>,
    flist=<optimized out>) at ../src/csync/csync_transfer.c:218
 #5  csync_pull_batch (info=0x7ff58400b880) at
../src/csync/csync_transfer.c:506
 #6  csync_xfer_backup (pipe=0x7ff58400b8e0, arg=0x7ff58400b880)
    at ../src/csync/csync_transfer.c:301
 #7  0x00007ff6629618d3 in ?? () from
/usr/lib/x86_64-linux-gnu/libczmq.so.4
 #8  0x00007ff6611ad4a4 in start_thread ()
   from /lib/x86_64-linux-gnu/libpthread.so.0
 #9  0x00007ff660eefd0f in clone () from /lib/x86_64-linux-gnu/libc.so.6

Fixed by doing a safe derefernce and checking for NULL.

VRVDR-54586

(cherry picked from commit 5605da3)
VyattaInternal pushed a commit that referenced this pull request Mar 3, 2021
npf: use rcu safe null check for sentry
7154ea2 
se_sen field in struct session may be NULL in two cases for newly
created sessions not yet added to sentry hash list, or after sesssion
removed from sentry hash list during reclaim in session gc.

This causes the following segmentaion fault infrequently.

[Current thread is 1 (Thread 0x7ff5c3fff700 (LWP 30235))]
 #0  0x000055ce31a8978d in csync_get_session_from_init_sentry (
    cse=<synthetic pointer>, cs=<synthetic pointer>, sp=0x7ff5f004ef36)
    at ../src/npf/csync/csync_session_unpack.c:38
 #1  csync_session_unpack_update (csu=0x7ff5f004ef2e)
    at ../src/npf/csync/csync_session_unpack.c:71
 #2  csync_unpack_session (size=<optimized out>, msg=0x7ff5f004ef26)
    at ../src/npf/csync/csync_session_unpack.c:421
 #3  csync_recv_session_update (frame=<optimized out>)
    at ../src/npf/csync/csync_session_unpack.c:501
 #4  0x000055ce31b2d57d in csync_restore_sessions (n=<optimized out>,
    flist=<optimized out>) at ../src/csync/csync_transfer.c:218
 #5  csync_pull_batch (info=0x7ff58400b880) at
../src/csync/csync_transfer.c:506
 #6  csync_xfer_backup (pipe=0x7ff58400b8e0, arg=0x7ff58400b880)
    at ../src/csync/csync_transfer.c:301
 #7  0x00007ff6629618d3 in ?? () from
/usr/lib/x86_64-linux-gnu/libczmq.so.4
 #8  0x00007ff6611ad4a4 in start_thread ()
   from /lib/x86_64-linux-gnu/libpthread.so.0
 #9  0x00007ff660eefd0f in clone () from /lib/x86_64-linux-gnu/libc.so.6

Fixed by doing a safe derefernce and checking for NULL.

VRVDR-54586

(cherry picked from commit 5605da3)
VyattaInternal pushed a commit that referenced this pull request Apr 9, 2021
cgnat: Split up cgn_map_get function #2
30ecf75 
Extract the block of code that assigns a new port-block from an already
assigned public address.  This new function is only used when the current
port-block of a subscriber has been fully allocated, which will only
occur very infrequently.  This is just moving of code.  There is no
functional change.
VRVDR-54775
VyattaInternal pushed a commit that referenced this pull request Oct 22, 2021
@srinivasknarayan
cgnat-alg-sip: Move fns from csip_parse_sip.c to csip_parse_utils.c #2
1ddfb29 
No actual code change.
VRVDR-54420
VyattaInternal pushed a commit that referenced this pull request Oct 22, 2021
@srinivasknarayan
cgnat-alg-sip: Move fns from csip_parse_sip.c to csip_request.c #2
9817993 
And to csip_response.c.  No actual code change.
VRVDR-54420
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@KaushikNiral KaushikNiral KaushikNiral left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hariosniral @KaushikNiral @nickbroon