Page MenuHomeFreeBSD

ipfw: Don't rollback state in alloc_table_vidx() if atomicity is not required.
ClosedPublic

Authored by nc on Dec 3 2019, 11:27 PM.

Details

Summary

ipfw: Don't rollback state in alloc_table_vidx() if atomicity is not required.

Submitted by: Neel Chauhan <neel AT neelc DOT org>

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

nc retitled this revision from Don't rollback state in alloc_table_vidx() if atomicity is not required. to ipfw: Don't rollback state in alloc_table_vidx() if atomicity is not required..Dec 3 2019, 11:28 PM
nc edited the summary of this revision. (Show Details)

Thank you for submitting the patch!

Looks good, please see some comments inline.

sys/netpfil/ipfw/ip_fw_table.c
657 ↗(On Diff #65193)

Could you please verify this is safe to insert the record w/o having a properly-linked value for this entry?
If yes, could you please add some comments on such input here?

sys/netpfil/ipfw/ip_fw_table_value.c
426 ↗(On Diff #65193)

Could you please verify that failed-to-be-added values are freed properly & update the comment wording to account for this use case?

Here are the comments. Sorry if they aren't perfect.

Would it be possible to provide details on testing these changes?

ip_fw_table.c
660 ↗(On Diff #65239)

So what will be the state of the system after executing this line for the item without valid value index?

sys/netpfil/ipfw/ip_fw_table.c
657 ↗(On Diff #65247)

Would it be possible if you could format the comment according to the style(9)?
Why XXX?

Fixed the comment. Sorry about that.

I also realized the XXX was unnecessary, so fixed that also.

This revision is now accepted and ready to land.Dec 9 2019, 10:16 PM