Hi Matt,

2011/1/17 mattschinkel <[email protected]>

> ***procedure icmp_send_echo_reply() (where there's a
> hardcoded network_set_remote_ip(192,168,1,10)
>
> oops, that was a mistake, I left it in there when I was doing
> troubleshooting. It can be commented out. Actually, that line has no
> affect at the moment.
>

Kutsal reported PIC was trying to reply to a certain 192.168.1.10....


>
> > 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 :)
>
> Thanks, you are defiantly helping with these thoughts. I'll see what I
> can do. Please continue :)
>

OK I'll try, while also trying to be less defiant :)


>
> About setting the IP every time...
> UDP has "sockets". When you create a socket/connection, the IP & port
> numbers you wish to use gets stored in an array that holds all socket
> information. If you have 5 connections, 5 ip's + port numbers will be
> stored. The library will set the ip address according to a socket name/
> number. Should I do the same for ping?
>

I not sure but, in your sample 18f4620_ping_udp_slip.jal (
http://code.google.com/p/jallib/source/browse/trunk/project/networking/18f4620_ping_udp_slip.jal?spec=svn2430&r=2430),
you have to define an array to store destination IP address, and, in your
loop, you also call network_set_remote_ip(192,168,2,2). This procedure, in
the end, does the same as when first defining the dest ip address. So, from
the code perspective, you're doing twice the same thing. Either you allow to
set an array, or you call network_set_remote_ip() procedure.

Both could feasible, but in the original sample&libs, you *had* to define
the byte array in order the sample to compile. Since then, it's been
deleted, things are moving fast :)

About UDP, indeed there are sockets, but it's a connectionless protocol, I'm
not sure you have to store some information, like some kind of session or
the like. When you receive a UDP datagram, don't you have every piece of
information to deal with it ?

I didn't have time to have a look at UDP, will do as soon as I can ping
google. With my PIC. Of course.


> At the moment, I think there is no reason to waste memory for ping,
> but I could be underestimating the importance of ping. Currently, you
> just send a ping to a specific ip address, and get one back. It does
> not check who is sending you a echo reply. With UDP, you would know
> exactly who reply's to your message. What do you think?
>

Not sure to understand. I guess it would better to fully honor ping spec, it
would a shame if your PIC was involved in a DOS attack :) Seriously, I think
it'd be better to setup a wiki page, or maybe just comments in the libs, to
track what's been implemented, what's not, what needs optimization, etc...


Defiant cheers,
Seb


> 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.

Reply via email to