[haiku-gsoc] Re: Fwd: Re: [HCD08] ICMP error handling & propagation project
- From: "Yin Qiu" <allenchue@xxxxxxxxx>
- To: haiku-gsoc@xxxxxxxxxxxxx
- Date: Thu, 31 Jul 2008 22:13:47 +0800
Hi Axel,
Thanks for your detailed comments and sorry for my unclear statements.
First I have modified my code according to your suggestions on coding
style. I also took a look at the coding style tips on the Haiku site.
>> 2. I'm not quite sure about my implementation of the error_reply
>> hooks.
>> Generally, ipv4_error_reply directly obtains the ICMP module info and
>> then
>> delegates the execution to icmp_error_reply. Would you please give me
>> some
>> advice on these two hooks?
>
> I'm not sure what you mean; at least the error_reply() code looks good
> to me.
Nice to hear that.
>> 3. Error handling mechanisms in TCP and UDP protocols are not yet
>> fully
>> implemented. However, maybe you can give some advice by reviewing the
>> skeleton code in tcp.cpp and udp.cpp.
>
> Not sure what advice you are seeking for. Do you mean how the protocol
> should react on the different errors?
Yes. Shall I consult relating rfc (maybe rfc793?) for this?
>> 5. Some invocation of error_reply currently lacks additional
>> parameter, such
>> as for ICMP type Parameter Problem. This will be resolved soon.
>
> Defining different NET_ERROR_* macros should be the way to go, or what
> do you intend to do about it?
Actually I was talking about the *errorData pointer value of the
error_reply hook. Currently all the error_reply's are invoked with
errorData being zero. Sometimes it may carry some data, e.g., for
Parameter Problem, it may indicate where the wrong octet is.
>> 6. Now that error_reply hook is a way to send ICMP error messages, I
>> don't
>> notice any function that takes care of sending ICMP query messages
>> like ECHO
>> and TIMESTAMP. Is there anything I missed? Or is icmp_send_data() for
>> this
>> purpose?
>
> Not sure if I understand you; what's wrong with how it behaves now, ie.
> in ICMP_TYPE_ECHO_REQUEST it does:
>
> status_t status = domain->module->send_data(NULL,
> reply);
> if (status < B_OK) {
> gBufferModule->free(reply);
> return status;
> }
>
My concern was, for instance, how do I send an TIMESTAMP request
message to a host using current API?
> BTW, I haven't tried to apply the patch, I just read through it - does
> it actually work, or have you just written it down without trying it
> yet? :-)
No, I haven't trying it out yet. It is now mostly a skeleton and is an
architecture work. Details are being added.
> as I said, useful NET_ERROR_* macros would be preferred.
I don't quite understand what you mean by defining NET_ERROR_* macros?
Could you show me an example? Anyway I want to explain my thoughts on
this.
Currently error_reply is called with the code argument being
icmp_encode(icmpType, icmpCode), where: icmp_encode(), defined in
icmp.h, encodes the ICMP type and code to another integer; icmpType
and icmpCode represent the ICMP type and code by their names and have
corresponding macros also defined in icmp.h, like ICMP_TYPE_UNREACH
and ICMP_CODE_PROTO_UNREACH.
The protocol indeed needs to know both the ICMP type and code. For
example, TCP should be able to know if an ICMP error is a host
unreachable message, a port unreachable message, or something else. So
I then decode the error code with icmp_decode() in erorr_received
hooks.
> I also don't see why the ICMP header is defined in public -- is it used
> anywhere outside the ICMP protocol module?
I noticed that there was no need for ICMP header struct to go public.
I'll move it back to icmp.cpp. However, IPv4 header is used inside the
ICMP module, so I have made it public.
--
Yin Qiu
Nanjing University, China
-------------------------------------------
Other related posts:
