Anything not quoted is completely satisfactory to me. Everything else mentioned is me verifying my understanding, and proposing one future-work-item.
On Wed, Aug 05, 2009 at 12:06:11PM -0400, Sebastien Roy wrote: <mucho snippage deleted!> > > DM-5 Lines 306-9 T5 Nit --> why EINVAL for both cases? > > One error is IPTUN_TYPE_IPV4, which perhaps > > could be EPROTONOSUPPORT? And the other is a > > too-large value... perhaps E2BIG? Or is an > > app expecting EINVAL for all of these? > > REJECT: The combination of the encapsulation limit property on a tunnel > type that is not IPv6 is invalid, thus EINVAL. This is in the context > of setting the "encaplimit" link property via dladm set-linkprop. If we > want to extend the DLADM_* error space to incorporate more specific > errors for property values that are out of range, or property values > that don't apply to the object they're being applied to, then that > should probably be done across the board for all properties and not just > this one. Okay, so the errnos aren't richer because the DLADM_* error space is not rich enough. Is that a correct understanding? <SNIP!> > > DM-8 Line 772 T5 If someone uses ndd and sets > > ip_path_mtu_discovery to off, should you be > > setting IPH_DF? > > DEFER: I need to figure out how this would work... It's much easier to > do this with Erik's refactoring code, which will follow this project. > Note that the tun module doesn't do that today, so not doing this > wouldn't be a regression... You're right -- it's not a regression. I was hoping we'd Do Better (TM) this time, but you're also right that datapath would be more appropriate for something like this. > > How will this play (or not) with TX? Have > > you had Jarrett or Ken look at this? > > TX doesn't play in exclusive stack zones. If this didn't work, then TCP > connections or any other IP clients represented by a conn_t wouldn't > work either with TX. Will have Ken or Jarrett look at this anyway; good > suggestion. And as you said at lunch, Will Young is also an excellent TX resource here. > > Finally, we've talked about split-zone > > tunnels (lower-layer/conn_t in one zone, > > upper-layer/GLDv3-stuff in another) -- will > > there need to be extra magic here if we pull > > this off? > > No, no magic needed, this already works. One can create a tunnel link > in the global zone and assign it to a non-global zone using zonecfg > and/or the "zone" link property. One can then plumb IP interfaces over > that tunnel link in the non-global zone. I've an RFE --> allow tunnel link *creation* in non-global zones. Then you can have Bill Sommerfeld's dream configuration of a VPN server where red (internal) and black (external) networks are completely distinct w.r.t. IP Instances, AND the black (external) network is on a non-global zone, just in case it IS compromised as well as the red (internal) one. <SNIP!> > > DM-11 Line 1378 T3 Looking from the contents of modhash.c, > > why > > are you locking iptun_hash at all? It > > appears that modhash.c's implementation > > enforces a reader/writer lock around this > > already. Is it because of the corner-case in > > iptun_task_cb()? You also mention a condvar > > up toward the top. That too? > > > > If either of those are the case, make sure > > you mention that at the top of iptun.c. > > ACCEPT: Will make this more clear in the locking notes that I need to > add. There is already a description of why this is needed in the > comments at 492-527, but this could be pulled up to a central location. When I said, "the corner-case in iptun_task_cb()", I meant lines 492-527, so I did catch that. Thanks for moving this up closer to the lock itself. Thanks! Dan
