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
Unknown Object (File)
Sat, Mar 30, 1:38 PM
Unknown Object (File)
Sat, Mar 30, 1:37 PM
Unknown Object (File)
Sat, Mar 30, 1:37 PM
Unknown Object (File)
Sat, Mar 30, 1:33 PM
Unknown Object (File)
Feb 27 2024, 7:21 PM
Unknown Object (File)
Jan 8 2024, 3:31 AM
Unknown Object (File)
Dec 20 2023, 2:59 AM
Unknown Object (File)
Dec 3 2023, 2:36 PM
View All 38 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 Not Applicable
Unit
Tests Not Applicable

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
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?

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 ↗(On Diff #65247)

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
head/
sys/
netpfil/
ipfw/
3 lines
10 lines
19 lines

Diff 65811

View Options

head/sys/netpfil/ipfw/ip_fw_table.h

Loading...
View Options

head/sys/netpfil/ipfw/ip_fw_table.c

Loading...
View Options

head/sys/netpfil/ipfw/ip_fw_table_value.c

Loading...