[Differential] [Commented On] D1986: Teach lagg(4) to change MTU

2016-01-06 Thread lakshmi.n_msystechnologies.com (LN)
lakshmi.n_msystechnologies.com added a comment. Thanks @smh, @melifaro and @hrs for the review comments. REPOSITORY rS FreeBSD src repository REVISION DETAIL https://reviews.freebsd.org/D1986 EMAIL PREFERENCES https://reviews.freebsd.org/settings/panel/emailpreferences/ To: rpokala,

[Differential] [Commented On] D1986: Teach lagg(4) to change MTU

2015-12-18 Thread hrs (Hiroki Sato)
hrs added inline comments. INLINE COMMENTS sys/net/if_lagg.c:753 Please separate a llq loop from a handler for per-port configuration. A llq traversal should be required only once in lagg_port_ops() if the handlers process a single lagg_llq entry. sys/net/if_lagg.c:837 Is this (llq ==

[Differential] [Commented On] D1986: Teach lagg(4) to change MTU

2015-12-17 Thread melifaro (Alexander V. Chernikov)
melifaro added inline comments. INLINE COMMENTS sys/net/if_lagg.c:728 What if we have multiple events queued on tasq? e.g mtu AND mac change sys/net/if_lagg.c:763 Not that easy, unfortunately. At this moment original ioctl returned 0, so other things/events were fired: rtsock

[Differential] [Commented On] D1986: Teach lagg(4) to change MTU

2015-12-17 Thread smh (Steven Hartland)
smh added a subscriber: smh. smh added a comment. Some general style nits and a question re-loss of the mtu set error. INLINE COMMENTS sys/net/if_lagg.c:706 style(9) bool use of pointer type. sys/net/if_lagg.c:731 style(9) four space additional indent only should be used, more below.

[Differential] [Commented On] D1986: Teach lagg(4) to change MTU

2015-10-28 Thread lakshmi.n_msystechnologies.com (LN)
lakshmi.n_msystechnologies.com added a comment. Thanks @hrs and @melifaro for the suggestions. I am working on the code changes to handle the MTU asynchronously. I will update the tested patch for review by this week. REPOSITORY rS FreeBSD src repository REVISION DETAIL

[Differential] [Commented On] D1986: Teach lagg(4) to change MTU

2015-10-27 Thread melifaro (Alexander V. Chernikov)
melifaro added a subscriber: melifaro. melifaro added a comment. After digging into lagg internals on updating lladdrs on lagg ports, I'd also vote for extenging llq to deal with MTU changes for underlying interfaces REPOSITORY rS FreeBSD src repository REVISION DETAIL

[Differential] [Commented On] D1986: Teach lagg(4) to change MTU

2015-10-19 Thread lakshmi.n_msystechnologies.com (LN)
lakshmi.n_msystechnologies.com added a comment. @sbruno, A gentle remainder on em driver's LOR issue. Can you please share your findings. REPOSITORY rS FreeBSD src repository REVISION DETAIL https://reviews.freebsd.org/D1986 EMAIL PREFERENCES

[Differential] [Commented On] D1986: Teach lagg(4) to change MTU

2015-10-19 Thread hrs (Hiroki Sato)
hrs added a subscriber: hrs. hrs added a comment. It is true that this LOR is driver-specific but calling SIOCSIFMTU after acquiring a lock in lagg ioctl is not always safe. Change of lladdr suffers from the same situation and it was solved by using an asynchronous task queue to update

[Differential] [Commented On] D1986: Teach lagg(4) to change MTU

2015-10-01 Thread sbruno (Sean Bruno)
sbruno added a subscriber: sbruno. sbruno added a comment. In https://reviews.freebsd.org/D1986#77466, @lakshmi.n_msystechnologies.com wrote: > We (Panasas) tried reproducing the problem by applying the patch and enabled > WITNESS in freebsd stable (10.1) kernel. > While testing with this

[Differential] [Commented On] D1986: Teach lagg(4) to change MTU

2015-10-01 Thread lakshmi.n_msystechnologies.com (LN)
lakshmi.n_msystechnologies.com added a subscriber: lakshmi.n_msystechnologies.com. lakshmi.n_msystechnologies.com added a comment. We (Panasas) tried reproducing the problem by applying the patch and enabled WITNESS in freebsd stable (10.1) kernel. While testing with this patched kernel, LOR

[Differential] [Commented On] D1986: Teach lagg(4) to change MTU

2015-03-05 Thread rpokala-panasas.com (Ravi Pokala)
rpokala-panasas.com added a comment. One of my colleagues dug into the first LOR - the one that happens when adding em1 to lagg0, which happens both with and without the patch. It looks like this is a known issue: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=194109 REVISION DETAIL

[Differential] [Commented On] D1986: Teach lagg(4) to change MTU

2015-03-02 Thread rpokala-panasas.com (Ravi Pokala)
rpokala-panasas.com added a comment. 4079 addresses @rstone's comments. However, the LOR remains: ``` Mar 2 17:00:43 fbsd-X root: ifconfig lagg0 mtu 9000 Mar 2 17:00:43 fbsd-X kernel: lock order reversal: Mar 2 17:00:43 fbsd-X kernel: 1st 0xf80004960c08 if_lagg rmlock (if_lagg rmlock) @

[Differential] [Commented On] D1986: Teach lagg(4) to change MTU

2015-03-01 Thread rpokala-panasas.com (Ravi Pokala)
rpokala-panasas.com added a comment. ! In D1986#4, @ae wrote: Just a thought. Imagine two interfaces, one has maximum MTU 2200, another 1500. lagg0 has MTU 1400. Two threads invokes changing MTU in the same time. One wants to change it to 2000, another - to 1500. It is possible, that when

[Differential] [Commented On] D1986: Teach lagg(4) to change MTU

2015-03-01 Thread rstone (Ryan Stone)
rstone added inline comments. INLINE COMMENTS sys/net/if_lagg.c:1772 style(9) says to not include unnecessary braces (which I personally disagree with, but what can you do?) sys/net/if_lagg.c:1773 style(9): put brackets around the return value: return (0); sys/net/if_lagg.c:1811 I

[Differential] [Commented On] D1986: Teach lagg(4) to change MTU

2015-03-01 Thread ae (Andrey V. Elsukov)
ae added a comment. ! In D1986#7, @rstone wrote: RLOCK only gets a read lock. You want WLOCK to get a write lock to ensure serialization. Also we can use another lock in the lagg_ioctl, that will prevent simultaneous MTU changing. REVISION DETAIL https://reviews.freebsd.org/D1986 To: