Hi Matt,
You're using network_data[] array to store layer-specific data. If I
understand well, network_data constains all the bits you'll finally send
over serial or any other physical media. Reading (quickly) the code, you're
filling this array, using an offset variable: when you're in ICMP (app
layer), network_var_offset matches the correct index within the array
(though it can be hard to follow as the network_var_offset is modified by
everybody everywhere, but anyway...).
So, back to our API thoughts... Let's consider icmp_send_echo_reply()
procedure. What I, the callee, would want to do when having to send a reply,
is to fill my part on network_data, and maybe set one or more flags
specifying current involved protocol is me (ICMP), then call the underlying
layer and let it does its job. What I do *not* want to is to deal with some
details I shouldn't be aware of, like knowing if ethernet is involved. My
contract is to forge the ICMP reply, that's all, and, well, that's quite a
job isn't ? :)
This would go something like this (not real code, I extracted the part I
think only corresponding to forge ICMP):
procedure icmp_send_echo_reply() is
(assuming offset is properly set, which may be hard to know if
previous/lower layers hasn't modified it yet)
-- set the ICMP header data
network_byte[ICMP_TYPE] = ICMP_ECHO_REPLY
network_word[ICMP_CHECKSUM] = 0
-- calculate and set the checksum
network_word[ICMP_CHECKSUM] =
network_checksum_16_byte_calc(network_var_offset,message_size)
current_protocol = ICMP -- "current_protocol" is not well named, it
should include the layer, maybe...
network_send_transport_data(...,...,...) -- some arguments, don't know
which
end procedure
Somewhere, network_send_transport_data is an alias of
network_send_ip_packet, because you're using IP. This procedure now has to
forge the IP packet, mostly including an IP header. It also fills its part
in network_data[].
procedure network_send_ip_packet(...,...,...)
-- set ip source, ip destination, call
ip_header_create(current_protocol,size)
-- give clue about involved IP protocol (here, IPv4)
network_send_datalink_data(..,,...,...)
end procedure
Somewhere, network_send_datalink_data is an alias of
network_send_ethernet_frame()
procedure network_send_ethernet_frame(...) is
-- forge ethernet frame, fill in part in network_data[], use previous
involved protocol to set ethertype field (IPv4)
network_send_data(...)
end procedure
network_send_data is an alias to whatever physical media you're using:
serial, etc... For Ethernet module, this procedure may just be a dummy one,
doing nothing, while letting the raw data being sent while in previous
procedure (you may not always be able to seperate datalink from physical
layer depending on module's API, we can't know for sure).
I hope you're still there, and if so, I hope you understand what I mean. You
actually have this kind of encapsulation, partly: you're filling
network_data[] in ICMP, except ethernet frame (but also deal with IP), then
let network_send_packet() deal with datalink stuff. What I'm suggesting is
to move this encapsulation scheme to the end, that is, dividing/splittting
actions by layers. Again these are very rough thoughts, it may not be
practical...
I can see at least two main problems:
- offset: my approach assume that, going up-to-down, you're able to know
the proper offset, matching corresponding data within network_data array.
Basically, your approach is to fill bottom-up, that is, from the beginning
of the array, to the end (except ethernet). This offset may be computed and
derived as you go upper, and may be hard to compute or guess the other way.
- alias: using aliases, you can specify for each layer, which
implementation to use (eg. datalink: slip or ethernet). Since we can't
define alias at runtime (or references/pointers to function), we won't be
able to handle different protocol on the same layer. Like a router using
SLIP and Ethernet :) Well, I'm half-joking, consider having this router
doing the same with Ethernet and ZigBee, that would be interested...
I'm not sure yet if this is the proper way to go. Implementing different
application layer protocols will help understand if it is or not...
HTH
Cheers,
Seb
2011/1/18 mattschinkel <[email protected]>
> I'm still trying to understand this. My approach is to call the lower
> layers first (MAC -> IP -> UDP). I think you are asking me to do it
> the other way around (UDP -> IP -> MAC)
>
> See icmp_send_echo()
>
> My method:
> 1. create a MAC address if needed (for ethernet only)
> 2. create an IP Header (always required by ICMP)
> 3. create ICMP data
> 4. send the data
>
> Your method:
> 1. create ICMP data,
> 2. send ICMP data to IP Header (ip header will require size and type
> code)
> 2. send icmp header + icmp data to ethernet MAC if ethernet, otherwise
> send the data
> 4. if slip send the data
>
> So, with your method, I will have to store a lot more variables so
> that I can pass them down to the next layer. With mine, all data goes
> directly into the network_data[] (the output array).
>
> Also, you can see at step 2 that IP Header requires some specific
> input parameters. Sending icmp data to something other then IP may
> require more or less parameters.
>
> Did I understand correctly?
>
> Isn't my flow readable?
>
> Matt.
>
> On Jan 17, 10:52 am, Sebastien Lelong <[email protected]>
> wrote:
> > Hi Matt,
> >
> > 2011/1/16 mattschinkel <[email protected]>
> >
> >
> >
> > > > - maybe ip_header.jal should also delegate higher protocol
> discovery to
> > > > something higher than IP. I understand why it checkes for ICMP (IP
> > > layer),
> > > > but I'm not sure about checking UDP (should it use transport.jal ?).
> > > Isn't
> > > > there a kind of abstraction leak here ?
> >
> > > Sorry, I'm not quite sure what you mean here. As mentioned above, it
> > > should not check for protocols you don't wish to use.
> >
> > What I mean is since TCP/IP is designed with layers, one layer should
> only
> > deal with its own protocols, and should not instead of a higher or lower
> > layer. This is at least what I'd do to implement this and take advantage
> of
> > such a layered design. What I mentioned was related to ip_header.jal lib,
> > where IP header is created and sets used protocol within the packet.
> Well,
> > that's not a good example, as its part of the specs (protocol field
> within
> > IP header defining payload type of a higher layer)
> >
> > But here's another example :)
> >
> > In icmp.jal @ rev. 2434, procedure icmp_send_echo_reply() (where there's
> a
> > hardcoded network_set_remote_ip(192,168,1,10) btw), you're checking if
> > ethernet is used as a link layer, and setting MAC address as needed.
> > According to me, this should be delegate to a lower layer, probably
> > implemented in ethernet_mac.jal (the test, not MAC address setting). Why
> ?
> > Because it's better concentrate code related to the same functionality
> into
> > the same place. Following current implementation, every new protocol will
> > have to check if ethernet is used or not (and this is the case, in
> udp.jal).
> > If you add another link layer protocol, you'll have to modify all
> libraries
> > related to higher layers.
> >
> > What's interesting is you actually check again this within
> network_main.jal
> > library, network_send_packet(). I would have put this test within this
> > procedure. Or, maybe better, add another level with network_send_frame().
> > Taking ICMP example:
> >
> > send_imcp()
> > - define payload (eg. ICMP echo).
> > - call send_packet
> >
> > (for udp:
> > send_udp()
> > - define payload
> > - can send_packet
> > )
> >
> > send_packet()
> > - possibly check if you're using IP (well, it's very probable, but...
> > maybe someone would be interested in implementing another one)
> > - set ip header, dest/src address, etc...
> > - call send_frame()
> >
> > send_frame()
> > - check link layer to use
> > - if ethernet: set ethernet header/trailer
> > - if slip: ...
> > - call send_data()
> >
> > As usual, this may not produce the most optimized code in terms of
> memory,
> > call stack, etc... But, as usual too, it may help in maintenance and
> > readability.
> >
> > These are just some thoughts, please correct me if I'm wrong, I have a
> very
> > narrowed view over your code. I hope it'll make sense too :)
> >
> > Cheers
> > Seb
>
> --
> You received this message because you are subscribed to the Google Groups
> "jallib" group.
> To post to this group, send email to [email protected].
> To unsubscribe from this group, send email to
> [email protected]<jallib%[email protected]>
> .
> For more options, visit this group at
> http://groups.google.com/group/jallib?hl=en.
>
>
--
Sébastien Lelong
http://www.sirloon.net
http://sirbot.org
--
You received this message because you are subscribed to the Google Groups
"jallib" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/jallib?hl=en.
