Page MenuHomeFreeBSD
Differential D5076

Fix regression introduced on 272446r
ClosedPublic
Actions

Authored by araujo on Jan 26 2016, 8:20 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Oct 16, 3:46 AM
Unknown Object (File)
Thu, Oct 16, 3:46 AM
Unknown Object (File)
Thu, Oct 16, 3:46 AM
Unknown Object (File)
Thu, Oct 16, 3:46 AM
Unknown Object (File)
Wed, Oct 15, 4:45 PM
Unknown Object (File)
Mon, Oct 13, 2:09 PM
Unknown Object (File)
Sun, Oct 12, 5:24 AM
Unknown Object (File)
Tue, Sep 23, 6:35 AM
View All Files
Subscribers
imp

Details

Reviewers
• rodrigc
hrs
glebius
rpokala
thompsa
bapt
Commits
rS295796: Fix regression introduced on 272446r.
Summary

(based on) PR: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=206478

lagg(4) supports the protocol none, where it disables any traffic without
disabling the lagg(4) interface itself.

Test Plan

ifconfig lagg0 create
ifconfig lagg0 laggproto none

Diff Detail

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

Event Timeline

araujo updated this revision to Diff 12709.Jan 26 2016, 8:20 AM
araujo retitled this revision from to Fix regression introduced on 272446r.
araujo updated this object.
araujo edited the test plan for this revision. (Show Details)
araujo added reviewers: • rodrigc, bapt.
Herald added a subscriber: imp. · View Herald TranscriptJan 26 2016, 8:20 AM
araujo added reviewers: hrs, glebius, thompsa.Jan 26 2016, 8:20 AM
araujo updated this revision to Diff 12932.Feb 1 2016, 5:50 PM

Rework the patch, now the statement looks more clean and
easy to understand.

bapt added inline comments.Feb 1 2016, 11:24 PM
sys/net/if_lagg.c
1268 ↗(On Diff #12932)

why not a single level of indentation?

araujo added inline comments.Feb 2 2016, 1:25 AM
sys/net/if_lagg.c
1268 ↗(On Diff #12932)

Do you mean a single level of statement? If it is about the statement.

For two reasons:

  1. I would prefer have the statement for the LAGG_PROTO explicit, much clear to understand, what is the intention there.
  1. Theoretically only one statement might be faster, but in the piece of code, the only way to know that is checking what the compiler will build with this statement. However, as this statement is not implemented in lagg_ioctl(), and it is not a fast path. It is OK to choose have more clarity in the code.
araujo added inline comments.Feb 2 2016, 1:41 AM
sys/net/if_lagg.c
1268 ↗(On Diff #12932)

Just rephrase here:

  1. Theoretically only one statement might be faster, but in the piece of code, the only way to know that is checking what the compiler will build with this statement. However, as this statement IS implemented in lagg_ioctl(), and it is not a fast path. It is OK to choose have more clarity in the code.

I just fix a misspelling that might make my affirmation confused.

bapt accepted this revision.Feb 4 2016, 12:18 PM
bapt edited edge metadata.
This revision is now accepted and ready to land.Feb 4 2016, 12:18 PM
rpokala accepted this revision.Feb 4 2016, 8:19 PM
rpokala added a reviewer: rpokala.
araujo abandoned this revision.Feb 15 2016, 4:53 AM

Abandon this revision in favor of: https://reviews.freebsd.org/D5226
That patch seemed much more reasonable and I made the tests it gets close with the same fix for 10-Stable.

Best,

Closed by commit rS295796: Fix regression introduced on 272446r. (authored by araujo). · Explain WhyFeb 19 2016, 6:36 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
head/
sys/
net/
2 lines

Diff 13485

View Options

head/sys/net/if_lagg.c

Loading...