odp icon indicating copy to clipboard operation
odp copied to clipboard

Published 20 hours ago • verified publisher icon OpenDataPlane

[PATCH v9] validation: ipsec: add sa expiry test

Open asasidharan opened this issue 3 years ago • 12 comments

Add IPsec SA expiry tests in out suite.

asasidharan avatar Feb 18 '22 09:02 asasidharan

FAIL: validation/api/pktio/pktio_run_netmap.sh

@MatiasElo, there seems to be a failure in one of the CI tests that looks unrelated to this PR. Could you please help me re-trigger the CI?

asasidharan avatar Feb 21 '22 11:02 asasidharan

FAIL: validation/api/pktio/pktio_run_netmap.sh

@MatiasElo, there seems to be a failure in one of the CI tests that looks unrelated to this PR. Could you please help me re-trigger the CI?

Hi @asasidharan, the netmap test randomly fails to create interfaces, which causes the faulure. I restarted the CI run.

MatiasElo avatar Feb 21 '22 11:02 MatiasElo

Hi @asasidharan, the netmap test randomly fails to create interfaces, which causes the faulure. I restarted the CI run.

Ack.

asasidharan avatar Feb 21 '22 11:02 asasidharan

@MatiasElo Seems like CI has failed for 3 checks, but I am not able to see any logs for the failing checks. I can see just a message saying "This check failed". Could you please help me understand why these checks failed?

image

asasidharan avatar Mar 30 '22 05:03 asasidharan

The checks failed due to GitHub Actions internal errors. You can see the error messages in the Summary view. Screen Shot 2022-03-30 at 10 02 30

Usually these errors are GitHub internal and unrelated to the tests. I restarted the failed three tests.

MatiasElo avatar Mar 30 '22 07:03 MatiasElo

Usually these errors are GitHub internal and unrelated to the tests. I restarted the failed three tests.

@MatiasElo Got it. Thank you for restarting the tests.

asasidharan avatar Mar 30 '22 07:03 asasidharan

@JannePeltonen I have made changes to the linux-generic implementation as suggested. Could you review the PR again?

asasidharan avatar May 04 '22 08:05 asasidharan

@JannePeltonen Ping.

asasidharan avatar May 23 '22 05:05 asasidharan

v8: fixed compilation issue introduced with new global variable sa_expiry_notified.

asasidharan avatar Jul 06 '22 06:07 asasidharan

@JannePeltonen ping!

asasidharan avatar Jul 25 '22 09:07 asasidharan

@JannePeltonen I have updated the PR with suggested changes. Could you please review the updated PR?

asasidharan avatar Sep 15 '22 07:09 asasidharan

@JannePeltonen @psavol This PR has been pending for quite some time. Can this be reviewed?

anoobj avatar Sep 27 '22 08:09 anoobj

@JannePeltonen @psavol Can this patch be reviewed?

anoobj avatar Nov 15 '22 06:11 anoobj

With v11:

  • linux-gen: renamed expiry_notified field to soft_expiry_notified
  • Updated commit message for 2nd commit and replaced usage of "cache" with "buffer"
  • Added comment to clarify the checking of error bits in IPsec result in case of SA expiry tests.

asasidharan avatar Nov 15 '22 15:11 asasidharan

With v12 updated the test to use zero wait time while receiving status event when sa_expiry is IPSEC_TEST_EXPIRY_IGNORED. Please review the updated PR.

asasidharan avatar Nov 16 '22 07:11 asasidharan

@JannePeltonen @psavol Can this PR be reviewed and closed before next release?

anoobj avatar Nov 17 '22 06:11 anoobj

I think the validation test code could be made cleaner as I commented and that might also help avoiding the current slowdown caused by waiting the status events. But since this PR has been going on for a long time and makes things better, even if not perfect, by adding missing validation tests and by fixing a bug in the implementation, I think it would probably be best to leave the improvements for later PRs and merge this in this form now. Thus I approved the PR now.

JannePeltonen avatar Nov 18 '22 12:11 JannePeltonen

@JannePeltonen Thank you for your time in reviewing the PR. I have added your "Reviewed-by" tag and rebased to the latest commit on master branch with v14.

asasidharan avatar Nov 21 '22 09:11 asasidharan