[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [MEXT] [WGLC] draft-ietf-mext-flow-binding-03
Hi Dave
Thanks a lot of your detailed review.
I am accepting most of your points below so see responses in line.
George
On Thu, Oct 8, 2009 at 1:37 AM, Dave Craig <dave.craig4 at gmail.com> wrote:
> Nice draft, I have a few comments that I hope will improve it a little bit.
>
> General comments, I do not see any specification of handling for
> fragments and ICMP responses. I think you need some text here because
> draft mentions filters applying to transport-specific headers and the
> desire not to reorder packets in a flow by sending them by different
> paths. I suggest you consider specifying HA behavior that attempts to
> direct fragments along the same binding as the previous fragment and
> ICMP responses along the path that corresponds to reversing the source
> and destination port of the packet encapsulated in an ICMP response.
> Please let me know if you'd like some text for this.
>
GT> Some good points here:
I am adding the following text, in the Packet Processing section :
"If a packet is fragmented, only the first fragment
contains all IP and transport layer headers, while
subsequent fragments only contain an IP header without
transport layer headers. For this reason it is possible
that subsequent fragments do not match the same traffic
selector as the initial fragment of such a packet.
Unless specific measures are taken the likely outcome is
that the initial fragment is routed as the MN intended
while subsequent fragments are routed differently, and
probably based on the default flow binding. HAs, MAPs,
and CNs SHOULD take care to forward all fragments of a
given packet the same way, and in accordance to the flow
binding matching the first fragment of said packet. This
should be possible given the fact that fragment headers
include enough information to identify a fragment as
part of a specific packet, but the details of how this
is ensured are implementation specific and are not
defined in this specification.
> I also suggest that the draft exclude flow bindings from impacting
> traffic originating from the home agent. It would not go very well if
> a node sends a discard all flow binding to temporarily stop traffic
> but is then unable to undo that flow binding.
>
GT> I am not sure it is worth saying something like that. The MN can
damage itself in many different ways, which we cannot really prevent
> All references to I-D.ietf-monami-multiplecoa should be replaced with
> RFC 5648, congratulations!
>
GT> done, thanks.
> * 2. Introduction, page 5, "...which binds one care-of address to one
> home address". Since you refer to NEMO, should this be, "...which
> binds one care-of address to one home address and possibly its
> associated Mobile Network"? I think this is germane to the flow
> mobility conversation because this draft enables migrating flows
> inside the mobile network independently of the home address.
>
GT> done
> * 2. Introduction, page 5, "A policy can contain a request for a
> special treatment...". This sounds better as, "A policy can contain a
> request for special treatment..."
>
GT> OK
> * 2. Introduction, page 5, "Requesting the flow binding can be decided
> based on local policies with the mobile node / mobile router and based
> on the link characteristics and the types of applications running at
> the time.". This sounds better as, "The mobile node / mobile router
> assembles the flow binding request based on local policies, link
> characteristics, and type of applications running at the time."
>
GT> done.
> * 2. Introduction, page 6, "with ant-replay protection mechanisms."
> should be, "with anti-replay..." The again, I may have overlooked an
> application for flow mobility in the pest extermination business.
>
GT> nice :-)
> * 2. Introduction, page 6, "Hence, per-packet load balancing is not
> currently supported in this extension." This is a non-sequitur with
> previous text mentioning the problems with load balancing. It sounds
> like you plan to support it at some point in the future. Suggest
> rephrasing with some text to effect that load balancing is not
> envisioned for this extension. Otherwise, as an implementer, I figure
> that I should start planning in extensibility for load balancing in my
> design.
>
GT> done.
> * 4.1. Definition Update for Binding Identifier Mobility Option, page
> 8, "BIDs with the same BID-PRI value have equal priority." Do you
> mean, "BIDs with the same BID-PRI produce unspecified results for
> flows matching both bindings."? It is not clear what equal priority
> means. An administrator may rightly assume a longest-prefix match for
> associated address-type filters, and I do not think you want to make
> that commitment here.
>
GT> added: "....meaning that, which BID is used is an implementation issue."
> * 4.2. Same comment for Flow Identification Mobility Option, page 9,
> with respect to FID-PRI.
>
GT> It does not apply here since as per WG request (despite authors'
disagreement) each FID MUST have a different FID-PRI
> * 4.2. Flow Identification Mobility Option, page 10, Discard. Should
> you specify whether there is an ICMP response when discarding the
> packet? Do you want two different actions, one that discards silently
> and one that discards with ICMP unreachable, network administratively
> prohibited? I think the later would be useful for p2p-type apps where
> you'd like to remove candidate paths as quickly as possible.
GT> Not so sure about this. I consider the discard function almost
like a firewall....firewalls typically silently discard afaik.
>
> * 4.2. Flow Identification Mobility Option, page 10. Should you
> include a "Forward" action to be consistent with the text in 5.3.2.1
> that mentions an 'Forward' action?
>
GT> no I will clean up text in 5.3.2.1, which is a remnant from last
version. Now we no longer have forward. We indicate it using n-cast
and pointing the flow to a single BID.
> * 4.2.1. Binding Reference Sub-option, page 11, do you need some error
> if the referenced BID does not exist? I noticed that errors for stale
> FID references had been specified but not BIDs.
>
GT> added "BID not found"
> * 4.4. Flow Bindings entries list and its relationship to Binding
> Cache, page 14, "A Flow Descriptor:..." Should this be Discriptor(s),
> since the format described in 4.2 allows multiple flow description
> sub-options for a single flow identification option?
>
GT> No, only one of these can be in a Flow Identification Option. I
will clarify.
> * 4.4. Flow Bindings entries list and its relationship to Binding
> Cache, page 14, "As an example the following represents an ordered
> flow bindings entries table..." Would this sound better as, "As an
> example, the following represents an ordered flow binding entry
> table..."
>
GT> OK
> * 4.4. Flow Bindings entries list and its relationship to Binding
> Cache, page 14, "A valid BID is required to make the entry "active"."
> This is inconsistent with Discard flow bindings that are not supposed
> to have a valid BID. How about requiring valid BID for discard flow
> bindings in 4.2, page 10 instead of updating this text? Otherwise,
> 5.2.4. removes flow bindings implicitly when there is a binding update
> that does not include them. If there is not a BID associated with a
> discard flow binding, how does the HA know which binding update
> excludes the flow binding? I guess the other alternative is to expand
> the flow binding entry table to include a reference to the binding
> update HoA and CoA that created it, but it is not clear that you want
> a n-cast type flow binding to disappear when removing the BU for
> HoA/CoA that created it.
>
GT> Good points. I would prefer not to reserve a BID for "Discard"
flow bindings. Instead, I will clarify that flow bindings that take
BIDs (i.e., other than those with "discard" action) need active BIDs
to be active. As already indicated for discard flow bindings, as well
as any other flow binding, they are removed by omitting the
corresponding FID from a subsequent BU. I will also clarify that
deleting BIDs renders flow "n-cast" flow bindings inactive, but does
nothing to "discard" flow bindings that in any case are not associated
with any BID.
> * 5.2.2.1. page 18, mentions a unique priority for each flow binding.
> Did you mean to limit a MN to at most 255 flow bindings? It probably
> will be less, as configurations will normally include space between
> priorities to make it easier to add new rules. Suggest treating this
> the same way as BID-PRI where the precedence between same priority is
> just (well should be) left unspecified in 4.1.
>
GT> That was the authors' proposal but we got overwritten by the WG. I
still do not like the unique priority per FID but I think it is
workable. Spaces are not really required since FIDs can be overwritten
at will (at the expense of longer BUs).
> * 5.2.2.1, page 18, "if the Action indicated 'Discard', the Binding
> Reference sub-option SHOULD NOT be included." How will a node know
> which binding update no longer includes this flow binding, if it does
> not have a BID? What do you think about removing this bullet?
>
GT> Flow bindings state is managed based on the FIDs, not the
BIDs....so the node will look at the FIDs to see which ones need to be
maintained.
> * 5.3.3. Receiving BU with Flow Summary Option, page 24, "A
> de-registration binding update (with a zero lifetime) would result in
> deleting all bindings regardless of the presence of the Flow summary
> option." Do you mean to remove all of the BID references in the flow
> bindings? At first glance, I thought this meant removing all of the
> bindings in the binding cache. There are several references to
> "bindings" in the document. Suggest a scrub for all instances of the
> word "binding" in the document, i.e. page 9, I think you meant to
> refer to flow bindings, not bindings, "This field is used to refer to
> an existing binding or to create a new binding."
>
GT> A dereg-BU removes ALL MIPv6 bindings, and all Flow Bindings so
5.3.3 is correct. You point about being careful with the use of the
term "binding" vs "flow binding" is well taken though... I went
through the document to qualified the term "binding" in several
additional places.
> * 5.2.4. Removing flow bindings, page 19-20, "Removal of flow binging
> entries is performed implicitly by omission of a given FID from a
> binding update". Change binging to binding. This really is, "A
> binding update removes its BID from a flow binding implicitly by
> omitting its FID in the Flow Identification Summary Mobility Option.
> Nodes remove flow binding entries when they no longer have any
> associated BIDs." The ensuing text that mentions removing BIDs
> associated with a FID references text that describes removing BIDs
> entirely, since RFC 5648 does not have the notion of a FID to
> selectively remove a particular BID. Suggest rewording to mention
> that it is possible to remove BIDs that also implicitly remove the BID
> references in the flow binding entry table. I also suggest that you
> also add text to remove BIDs from flow bindings when the binding for a
> particular BID expires.
>
GT> I think the draft is clear on this:.
a) BU deregistration (RFC3775) removes ALL state, including BID and FID stated
b) BID deregistration (RFC5648) removes a specific BID
c) In Flow Binding Removals:
- If all BIDs associated with an n-cast flow binding are
removed the the flow binding becomes inactive
- Flow bindings are deleted by omitting their FIDs from next BU.
In any case I am rewording the sections you refer to above as per your
earlier comments which I think will make this clearer.
> * 5.3.2. page 22, "If more than one flow identification option," is,
> "in the same BU..."
>
GT> actualy it should say: If more than one flow identification
options in the same
BU have the same value in the FID field, all the flow
identification options MUST be rejected."
> * 5.3.2.1. page 23, "If the Binding Reference sub-option is present
> and includes one or more BIDs..." Should you reject a ncast flow
> binding that refers to bindings that are not active? Wouldn't this
> diminish the utility of ncast if the HA removes a binding that is not
> refreshed and the MN sends a refresh referencing that binding? In
> this case, the n-cast goes to 0-cast even though n-1 bindings are ok.
> Suggest removing this bullet or at least paring it down to an error if
> all of the referenced BIDs do not exist.
>
GT> I am not sure I following you. This only says that when you
create a flow binding all the information must be valid, including the
BIDs you are listing. I think this is just normal expectation, no?.
> * 5.3.3. page 24, "If the value of the FID field is present in the
> mobile nodes list of slow binding entries the, home agent SHOULD
> refresh the binding entry identified by the FID..." Suggest removing
> the refresh term. There was not a lifetime specified in the flow
> binding entry table. This might be more consistent as, "... home
> agent SHOULD not remove the BIDs associated with the identified
> FID..."
>
GT> Ah, this is again an remnant of an old version. We no longer make
any distinction between add and modify requests in the messages. This
is purely done based on whether an FID in a BU already exists in the
HA or not. If it exist then the entry associated with the FID is
modified according to the contents of the BU, oif the FID does not
exist then a new entry is added. I will clean up this part of the spec
appropriately.
> * 5.3.5. page 25, "... or by just ignoring all the flow binding
> options." Isn't this inconsistent with the text requiring flow
> identification options MUST be included in the Binding Acknowledgment?
> Suggest dropping the ignoring text or demote the MUST requirement to
> a SHOULD.
>
GT> You are right, I changed MUST to SHOULD.
_______________________________________________
> MEXT mailing list
> MEXT at ietf.org
> https://www.ietf.org/mailman/listinfo/mext
>