Subject: Re: comments on ipngwg-rfc2292bis-01

From: JINMEI Tatuya / 神明達哉<[email-masked]>
Date: Tue, 01 Feb 2000 23:38:03 +0900

Sorry for the delayed response.

>>>>> On Thu, 27 Jan 2000 20:31:37 +1100, 
>>>>> Robert Elz <[email-masked]> said:

>     Date:        Thu, 27 Jan 2000 11:38:51 +0900
>     From:        JINMEI Tatuya / =?ISO-2022-JP?B?GyRCP0BMQEMjOkgbKEI=?=
> 		 <[email-masked]>
>     Message-ID:  <[email-masked]>

>   | Of course yes in theory, but I'd think that an IPv6 extension header
>   | rarely needs a length field in more than one byte.

> That depends what they end up being used for - I wouldn't like to define
> something now which would constrain choices in the future.   The (not
> accepted but for this purpose that doesn't matter) payload header intended
> to allow a UCP or TCP (or anything else) packet to appear in its data
> field - clearly a one byte length would never have worked.

Okay, I understand your concern. I still believe the structure is
useful, but I don't stick to include it in the draft.

>   | Sorry, the sample code was too simple. In our actual code we of course
>   | reject an unknown header.

> That's good.   Then, when you stop parsing the packet when you reach the
> unknown header, there's no need to know its length - because there's no
> need to skip over it (or do anthing else at all to it).  Packet processing
> simply stops (and an ICMPv6 can be returned - that also doesn't need to be
> able to parse the insides of the unknown header, or anything beyond it).

Right, so the proposed structure is for parsing known headers in a
generic way. That is, I prefer code like

		while (len < off) {
			ip6e = (struct ip6_ext *)(packetbuf + len);

			switch(nxt) {
			case IPPROTO_FRAGMENT:
				len += sizeof(struct ip6_frag);
				break;
			case IPPROTO_AH:
				len += (ip6e->ip6e_len + 2) << 2;
				break;
			case IPPROTO_HOPOPTS:
			case IPPROTO_DSTOPTS:
			case IPPROTO_ROUTING:
				len += (ip6e->ip6e_len + 1) << 3;
				break;
			default:
				/* impossible case. discard and/or return
			           an error */
				break;
			}
			nxt = ip6e->ip6e_nxt;
		}

instead of the following:

		struct ah *ah;
		struct ip6_hbh *hbh;
		struct ip6_dest *dest;
		struct ip6_rthdr *rthdr;
		struct ip6_frag *frag;

		while (len < off) {
			ip6e = (struct ip6_ext *)(packetbuf + len);

			switch(nxt) {
			case IPPROTO_FRAGMENT:
				frag = (struct ip6_frag *)(packetbuf + len);
				len += sizeof(struct ip6_frag);
				nxt = frag->ip6f_nxt;
				break;
			case IPPROTO_AH:
				ah = (struct ah *)(packetbuf + len);
				len += (ah->ah_len + 2) << 2;
				nxt = ah->ah_nxt;
				break;
			case IPPROTO_HOPOPTS:
				hbh = (struct ip6_hbh *)(packetbuf + len);
				len += (hbh->ip6h_len + 1) << 3;
				nxt = hbh->ip6e_nxt;
				break;
			case IPPROTO_DSTOPTS:
				....(omitted)
			case IPPROTO_ROUTING:
				....(omitted)
			default:
				/* impossible case. discard and/or return
			           an error */
				break;
			}
		}

(BTW: the both examples assume that we have already parsed all the
      headers correctly and no validation is necessary.)

But again, I don't argue it should be in the draft anymore.

					JINMEI, Tatuya
					Communication Platform Lab.
					Corporate R&D Center, Toshiba Corp.
					[email-masked]
--------------------------------------------------------------------
IETF IPng Working Group Mailing List
IPng Home Page:                      http://playground.sun.com/ipng
FTP archive:                      ftp://playground.sun.com/pub/ipng
Direct all administrative requests to [email-masked]
--------------------------------------------------------------------