Page MenuHomeFreeBSD
Differential D22662

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

Authored by nc on Dec 3 2019, 11:27 PM.
Tags
None
Referenced Files
F103508835: D22662.diff
Mon, Nov 25, 9:46 PM
Unknown Object (File)
Sun, Nov 24, 4:09 PM
Unknown Object (File)
Sun, Nov 24, 4:07 PM
Unknown Object (File)
Wed, Nov 20, 8:06 PM
Unknown Object (File)
Wed, Nov 6, 1:51 AM
Unknown Object (File)
Fri, Nov 1, 2:53 AM
Unknown Object (File)
Oct 20 2024, 10:46 AM
Unknown Object (File)
Oct 10 2024, 11:05 AM
View All 86 Files
Subscribers
ae
imp
melifaro
Contributor Reviews (src)

Details

Reviewers
ae
melifaro
Commits
rS355910: ipfw: Don't rollback state in alloc_table_vidx() if atomicity is not required.
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 Skipped
Unit
Tests Skipped

Event Timeline

nc created this revision.Dec 3 2019, 11:27 PM
Herald added subscribers: Contributor Reviews (src), ae, imp. · View Herald TranscriptDec 3 2019, 11:27 PM
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)
nc updated this revision to Diff 65193.Dec 4 2019, 12:08 AM
melifaro added a subscriber: melifaro.Dec 4 2019, 11:12 AM

Thank you for submitting the patch!

Looks good, please see some comments inline.

sys/netpfil/ipfw/ip_fw_table.c
663

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
425–426

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

melifaro added reviewers: ae, melifaro.Dec 4 2019, 11:12 AM
nc updated this revision to Diff 65239.Dec 4 2019, 8:14 PM

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

melifaro added a comment.Dec 4 2019, 8:23 PM

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?

nc updated this revision to Diff 65247.Dec 4 2019, 10:24 PM
melifaro added inline comments.Dec 8 2019, 9:50 AM
sys/netpfil/ipfw/ip_fw_table.c
657

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

nc updated this revision to Diff 65407.Dec 8 2019, 12:45 PM

Fixed the comment. Sorry about that.

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

melifaro accepted this revision.Dec 9 2019, 10:16 PM
This revision is now accepted and ready to land.Dec 9 2019, 10:16 PM
Closed by commit rS355910: ipfw: Don't rollback state in alloc_table_vidx() if atomicity is not required. (authored by melifaro). · Explain WhyDec 19 2019, 10:22 AM
This revision was automatically updated to reflect the committed changes.
melifaro added a commit: rS355910: ipfw: Don't rollback state in alloc_table_vidx() if atomicity is not required..

Revision Contents
Changeset List

PathSize
sys/
netpfil/
ipfw/
3 lines
8 lines
19 lines

Diff 65247

View Options

sys/netpfil/ipfw/ip_fw_table.h

Loading...
View Options

sys/netpfil/ipfw/ip_fw_table.c

Loading...
View Options

sys/netpfil/ipfw/ip_fw_table_value.c

Loading...