rtnet-developers — development-related list

Re: [RTnet-developers] Problem in SOCK_RAW

[Jan K. <jan...@we...>]

Jorge Almeida wrote:
> Hello to all,
>=20
> I'm testing the SOCK_RAW functionality in the rtnet framework for long =
periods of time and a problem is happening.
> I will try to describe it:
>=20
> I'm making tests sending 1.000.000 (one milion messages), with an inter=
val of 5 ms each.
> After some time, more than 100.000 messages, the host were the test is =
running has a strange behaviour, the program does not return but the bash=
 dies. I must make the login phase again to enter the host.
> The messages stop of being sent (I'm monitoring the network with ethere=
al).
>=20
> In the /proc i find some data about the file descriptor used by the soc=
ket (/proc/rtai/rtdm/open_fildes)
> Index   Locked   Device
> 0         0            PACKET_RAW
>=20
> I think this is OK because the socket was never closed.

Because the sender somehow died I think. But why does the console also
die? That's not a typical program error. Anything on the kernel console?

>=20
> But the behaviour is strange.
> My guess is that this problem is due to some kind of semaphore or any s=
ynchronization mechanism.

The guess is based on which information?

>=20
>=20
> Any clues for wath is happening?

Nope.

If there are no signs anywhere, I would first try to run your scenario
over a similar time using some vanilla RTnet version with normal packet
sockets. Have you tried this before? Just to exclude that there are
major stability issues.

BTW, you are on RTAI? What version, patch, gcc?

Jan

[RTnet-developers] Problem in SOCK_RAW

[Jorge A. <j-a...@cr...>]

Hello to all,

I'm testing the SOCK_RAW functionality in the rtnet framework for long periods of time and a problem is happening.
I will try to describe it:

I'm making tests sending 1.000.000 (one milion messages), with an interval of 5 ms each.
After some time, more than 100.000 messages, the host were the test is running has a strange behaviour, the program does not return but the bash dies. I must make the login phase again to enter the host.
The messages stop of being sent (I'm monitoring the network with ethereal).

In the /proc i find some data about the file descriptor used by the socket (/proc/rtai/rtdm/open_fildes)
Index   Locked   Device
0         0            PACKET_RAW

I think this is OK because the socket was never closed.

But the behaviour is strange.
My guess is that this problem is due to some kind of semaphore or any synchronization mechanism.


Any clues for wath is happening?

Thanks

-- 
Jorge Almeida
j-a...@cr...
DISCLAIMER: This message may contain confidential information or privileged material and is intended only for the individual(s) named. If you are not a named addressee and mistakenly received this message you should not copy or otherwise disseminate it: please delete this e-mail from your system and notify the sender immediately. E-mail transmissions are not guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete or contain viruses. Therefore, the sender does not accept liability for any errors or omissions in the contents of this message that arise as a result of e-mail transmissions. Please request a hard copy version if verification is required. Critical Software, SA.

Re: [RTnet-developers] Patches for SOCKET_RAW and ETH_P_ALL

[Jan K. <jan...@we...>]

Jorge Almeida wrote:
> New Patch.
>=20
> Please check if everything is now OK.

Well, the thrilling question is now if it works... Did you test it alread=
y?

If it does, the rest is about introducing the ETH_P_ALL subscriber list
and coding style cleanups (alignments with the RTnet style).

Jan

Re: [RTnet-developers] Patches for SOCKET_RAW and ETH_P_ALL

[Jan K. <jan...@we...>]

Jorge Almeida wrote:
> Hi Jan
> Only now i have time to look at your sugestions.
>=20
> I make some comments below and send a new patch against SVN HEAD for yo=
u to look and fix some things that i comment below.
>=20
=2E..
>>> Index: stack/stack_mgr.c
>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>>> --- stack/stack_mgr.c	(revision 1060)
>>> +++ stack/stack_mgr.c	(working copy)
>>> @@ -27,6 +27,7 @@
>>>  #include <rtnet_internal.h>
>>>  #include <rtskb_fifo.h>
>>>  #include <stack_mgr.h>
>>> +#include <rtnet_socket.h>
>> See my comment on the SOCK_RAW patch (fatal layering violation...)
>>
>>> =20
>>> =20
>>>  static unsigned int stack_mgr_prio =3D RTNET_DEF_STACK_PRIORITY;
>>> @@ -54,15 +55,13 @@
>>>      int                     ret =3D 0;
>>>      rtdm_lockctx_t          context;
>>> =20
>>> -
>>> -    if (pt->type =3D=3D htons(ETH_P_ALL))
>>> +    if (likely(pt->type =3D=3D htons(ETH_P_ALL) && (strncmp(pt->name=
,PACKET_SOCKET,PACKET_SOCKET_LENGTH) !=3D 0 )))
>>>          return -EINVAL;
>> Please explain: what's the idea of this string comparison?
> The string comparison is to protect the socket type. Only the PACKET_SO=
CKET is prepared to clone the rtskb and proceed with the correct way of t=
hings.
> I don't know any other kind of identifying without mistakes the PACKET_=
SOCKET with the available information. Maybe you do??!!
> Please comment.

Check the existing code (e.g. [1]): sockctx->device->socket_type

>>> @@ -161,33 +157,46 @@
>>>              skb =3D __rtskb_fifo_remove(&rx.fifo);
>>>              if (!skb)
>>>                  break;
>>> -
>>>              rtdev =3D skb->rtdev;
>>> =20
>>>              rtcap_report_incoming(skb);
>>> =20
>>>              skb->nh.raw =3D skb->data;
>>> =20
>>> +            /*hash for the ETH_P_ALL packets*/
>>> +            hash =3D ETH_P_ALL & RTPACKET_HASH_KEY_MASK;
>>> +            rtdm_lock_get_irqsave(&rt_packets_lock, context);
>>> +            /*for cycle that runs over all list entries to check if =
anyone has ETH_P_ALL for protocol type*/
>>> +            list_for_each_entry(pt_entry, &rt_packets[hash], list_en=
try)
>> It's better to keep ETH_P_ALL separated from normal subscribers. If so=
me
>> protocol matches the hash key of ETH_P_ALL, we but that subscriber
>> without any needs into this hotpath.
> I don't understand this comment.
> Another list only with ETH_P_ALL sockets????????!!!!!!!!!!!!!!!!!!!

Exactly.

>>> -
>>> +            /*for cycle that runs over all list entries*/
>>>              list_for_each_entry(pt_entry, &rt_packets[hash], list_en=
try)
>>>                  if (pt_entry->type =3D=3D skb->protocol) {
>>>                      pt_entry->refcount++;
>>>                      rtdm_lock_put_irqrestore(&rt_packets_lock, conte=
xt);
>>> -
>>>                      err =3D pt_entry->handler(skb, pt_entry);
>>> -
>>>                      rtdm_lock_get_irqsave(&rt_packets_lock, context)=
;
>>>                      pt_entry->refcount--;
>>>                      rtdm_lock_put_irqrestore(&rt_packets_lock, conte=
xt);
>>> -
>>>                      rtdev_dereference(rtdev);
>>>                      if (!err)
>>>                          goto next_packet;
>>>                  }
>>> -
>> Please kill all those code layout changes like the above. They exist t=
o
>> improve the readability.
> Until the last patch, stable version, i think is better to keep it.

Don't understand your argumentation. The larger your patch becomes, the
more functionally irrelevant changes it contains, the harder it is to
maintain (and to review).

>>> Index: stack/rtskb.c
>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>>> --- stack/rtskb.c	(revision 1060)
>>> +++ stack/rtskb.c	(working copy)
>>> @@ -483,8 +483,34 @@
>>>      return 0;
>>>  }
>>> =20
>>> +/** Clone a rtskb to another, allocating a rtskb for the copy  */
>>> +int rtskb_clone(struct rtskb *rtskb_1,struct rtskb *rtskb_2,struct r=
tskb_queue *pool)
>> Please rename, e.g. rtskb_1 -> rtskb_new, rtskb_2 -> rtskb.
> Renamed
>>> +{
>>> +    if(rtskb_1 =3D=3D NULL)
>>> +        return -ENOMEM;
>> Mmh, wouldn't it make sense to also allocate the rtskb here and return=

>> the pointer? That's what Linux does as well. The prototype would then
>> look like this:
>>
>> struct rtskb *rtskb_clone(struct rtskb *rtskb, struct rtskb_queue *poo=
l)
>>
> DONE
>=20
>>> =20
>>> +    rtskb_1->chain_end  =3D rtskb_1;
>> Done on allocation - unless you manage rtskbs in your own secrete queu=
e
>> (which you don't).
>>
> DONE
>>> +    memcpy(rtskb_1->data, rtskb_2->data, rtskb_2->len );
>> That looks wrong. The original rtskb may have already forwarded its da=
ta
>> pointer past the MAC header, and the same must happen to the clone. Yo=
u
>> have to account for the current data vs. the mac.raw, copy the full
>> block to the target, and then move the data pointer of the target
>> accordingly.
>>
>=20
> I don't understand correctly this. The mac.raw pointer has the all mess=
age? I thought that it contains only the Ethernet header!

mac.raw always points to the beginning of a frame (because one assumes
the mac layer adds its stuff to the frame head - fairly common...).
mac.eth is just a typecast to the Ethernet header, see the code!

Now to the new one (already quite short, that's a good sign :)):

> -----------------------------------------------------------------------=
-
>=20
> Index: stack/include/rtskb.h
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- stack/include/rtskb.h	(revision 1063)
> +++ stack/include/rtskb.h	(working copy)
> @@ -728,6 +728,7 @@
>  extern unsigned int rtskb_pool_shrink_rt(struct rtskb_queue *pool,
>                                           unsigned int rem_rtskbs);
>  extern int rtskb_acquire(struct rtskb *rtskb, struct rtskb_queue *comp=
_pool);
> +extern struct rtskb* rtskb_clone(struct rtskb *rtskb,struct rtskb_queu=
e *pool);
> =20
>  extern int rtskb_pools_init(void);
>  extern void rtskb_pools_release(void);
> Index: stack/stack_mgr.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- stack/stack_mgr.c	(revision 1063)
> +++ stack/stack_mgr.c	(working copy)
> @@ -55,7 +55,7 @@
>      rtdm_lockctx_t          context;
> =20
> =20
> -    if (pt->type =3D=3D htons(ETH_P_ALL))
> +    if (likely(pt->type =3D=3D htons(ETH_P_ALL) && (strncmp(pt->name,"=
PACKET_SOCKET",13) !=3D 0 )))

Why do you think this should be limited to packet sockets? Anyone
subscribing to ETH_P_ALL would, of course, have to take care of the
implication this has just like you already do in af_packet.c. But this
sanity check makes no sense here...

>          return -EINVAL;
> =20
>      INIT_LIST_HEAD(&pt->list_entry);
> @@ -84,7 +84,7 @@
> =20
>      RTNET_ASSERT(pt !=3D NULL, return -EINVAL;);
> =20
> -    if (pt->type =3D=3D htons(ETH_P_ALL))
> +    if (likely(pt->type =3D=3D htons(ETH_P_ALL) && (strncmp(pt->name,"=
PACKET_SOCKET",13) !=3D 0 )))

=2E..and here.

>          return -EINVAL;
> =20
>      rtdm_lock_get_irqsave(&rt_packets_lock, context);
> @@ -167,11 +167,26 @@
>              rtcap_report_incoming(skb);
> =20
>              skb->nh.raw =3D skb->data;
> -
> +/* */       =20
> +            /*hash for the ETH_P_ALL packets*/
> +            hash =3D ETH_P_ALL & RTPACKET_HASH_KEY_MASK;
> +            rtdm_lock_get_irqsave(&rt_packets_lock, context);
> +            /*for cycle that runs over all list entries to check if an=
yone has ETH_P_ALL for protocol type*/
> +            list_for_each_entry(pt_entry, &rt_packets[hash], list_entr=
y)
> +            {
> +                if ( likely(ntohs(pt_entry->type) =3D=3D hash &&=20
> +                     (strncmp(pt_entry->name,"PACKET_SOCKET",13) =3D=3D=
 0 )))
> +                {
> +                    pt_entry->refcount++;
> +                    rtdm_lock_put_irqrestore(&rt_packets_lock, context=
);
> +                    err =3D pt_entry->handler(skb, pt_entry);
> +                    rtdm_lock_get_irqsave(&rt_packets_lock, context);
> +                    pt_entry->refcount--;
> +                }
> +            }
> +/* */

TODO: separate ETH_P_ALL subscriber list.

BTW, I just realised that you will need more patching below to avoid
massive "RTnet: no one cared for packet..." warnings if there are
ETH_P_ALL listeners but no one on the regular list for that specific
protocol. So, track if there was at least one ETH_P_ALL listener
accepting the frame and skip the warning in that case.

>              hash =3D ntohs(skb->protocol) & RTPACKET_HASH_KEY_MASK;
> =20
> -            rtdm_lock_get_irqsave(&rt_packets_lock, context);
> -
>              list_for_each_entry(pt_entry, &rt_packets[hash], list_entr=
y)
>                  if (pt_entry->type =3D=3D skb->protocol) {
>                      pt_entry->refcount++;
> Index: stack/rtskb.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- stack/rtskb.c	(revision 1063)
> +++ stack/rtskb.c	(working copy)
> @@ -483,8 +483,39 @@
>      return 0;
>  }
> =20
> +/** Clone a rtskb to another, allocating a rtskb for the copy  */
> +struct rtskb* rtskb_clone(struct rtskb *rtskb,struct rtskb_queue *pool=
)
> +{
> +    struct rtskb* clone_rtskb =3D NULL;
> +    clone_rtskb =3D alloc_rtskb(rtskb->len,pool);
> +    if(clone_rtskb =3D=3D NULL)
> +        return clone_rtskb;
> +    /* Please check this */
> +    memcpy(clone_rtskb->data, rtskb->data, rtskb->len );

memcpy(clone_rtskb->data, rtskb->mac.raw,
       rtskb->len + rtskb->data - rtskb->mac.raw);

> +   =20
> +    clone_rtskb->len        =3D rtskb->len;
> +    clone_rtskb->time_stamp =3D rtskb->time_stamp;
> +    clone_rtskb->rtdev      =3D rtskb->rtdev;
> +    clone_rtskb->protocol   =3D rtskb->protocol;
> =20
> +    clone_rtskb->pkt_type   =3D rtskb->pkt_type;
> +    clone_rtskb->priority   =3D rtskb->priority;
> +    clone_rtskb->ip_summed  =3D rtskb->ip_summed;
> +    clone_rtskb->csum       =3D rtskb->csum;
> +   =20
> +    /* Please check this */
> +    /*copy ethernet stuff*/
> +    clone_rtskb->mac.raw    =3D clone_rtskb->data;

Yes, but you also have to issue:

__rtskb_pull(clone_rtskb, rtskb->data - rtskb->mac.raw);

This forwards clone_rtskb->data to the same relative position like
rtskb->data.

> +    memcpy(clone_rtskb->mac.ethernet->h_dest,rtskb->mac.ethernet->h_de=
st, ETH_ALEN);
> +    memcpy(clone_rtskb->mac.ethernet->h_source,rtskb->mac.ethernet->h_=
source, ETH_ALEN);
> +    clone_rtskb->mac.ethernet->h_proto =3D rtskb->mac.ethernet->h_prot=
o;

Not needed with the full copy above.

> +   =20
> +    return clone_rtskb;
> +}

[No guarantee that me quick review didn't oversee some detail. This
cloning is certainly not trivial.]

> =20
> +
> +
> +
>  int rtskb_pools_init(void)
>  {
>      rtskb_slab_pool =3D kmem_cache_create("rtskb_slab_pool",
> @@ -547,6 +578,7 @@
>  EXPORT_SYMBOL(global_pool);
> =20
>  EXPORT_SYMBOL(rtskb_acquire);
> +EXPORT_SYMBOL(rtskb_clone);
> =20
>  #ifdef CONFIG_RTNET_CHECKED
>  EXPORT_SYMBOL(rtskb_over_panic);
> Index: stack/packet/af_packet.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- stack/packet/af_packet.c	(revision 1063)
> +++ stack/packet/af_packet.c	(working copy)
> @@ -43,16 +43,34 @@
>      void            (*callback_func)(struct rtdm_dev_context *, void *=
);
>      void            *callback_arg;
>      rtdm_lockctx_t  context;
> +    struct rtskb    *clone_skb =3D NULL;
> +    unsigned short  hash =3D 0;
> +    int             err =3D 0;
> =20
> =20
>      if (unlikely((ifindex !=3D 0) && (ifindex !=3D skb->rtdev->ifindex=
)))
>          return EUNATCH;
> -
> -    if (unlikely(rtskb_acquire(skb, &sock->skb_pool) !=3D 0))
> -        kfree_rtskb(skb);
> +    /*hash for the ETH_P_ALL packets*/
> +    hash =3D ETH_P_ALL & RTPACKET_HASH_KEY_MASK;
> +    if(likely(ntohs(pt->type) =3D=3D hash &&=20

Why not (pt->type =3D=3D htons(ETH_P_ALL))?

> +       (strncmp(pt->name,"PACKET_SOCKET",13) =3D=3D 0 )))

Redundant, we are already in af_packet.c.

> +    {
> +        clone_skb =3D rtskb_clone(skb, &sock->skb_pool);
> +        if( clone_skb =3D=3D NULL)
> +            return 0;

I would prefer

skb =3D clone_skb;

so that...

> +    }
> +    else
> +    {
> +        err =3D rtskb_acquire(skb, &sock->skb_pool);
> +        clone_skb =3D skb;
> +    }
> +    if ( err !=3D 0)
> +    {
> +        kfree_rtskb(clone_skb);
> +    }
>      else {
> -        rtdev_reference(skb->rtdev);
> -        rtskb_queue_tail(&sock->incoming, skb);
> +        rtdev_reference(clone_skb->rtdev);
> +        rtskb_queue_tail(&sock->incoming, clone_skb);

=2E..such changes are not required.

>          rtdm_sem_up(&sock->pending_sem);
> =20
>          rtdm_lock_get_irqsave(&sock->param_lock, context);

We are getting closer, much closer. :)

Jan

Re: [RTnet-developers] Test programs for SOCK_RAW and ETH_P_ALL

[Jan K. <jan...@we...>]

Jorge Almeida wrote:
> Jan,
>=20
> In attach follows the test cases for the SOCK_RAW and ETH_P_ALL feature=
s. Its two files written in C code.
> One for sending messages, and other to read they using the ETH_P_ALL fe=
ature.

Thanks, will have a look.

>=20
> Please test the patch i send to you, after the refactoring that you nee=
d to do, and comment.

Well, if you read my comment, this is first of all about fixing your
patch. Therefore I would appreciate some comments on my findings as well
(disagree, ack, answers to my questions...). Even better would be an
updated version that is at least correct wrt/ rtskb_clone and resolves
the layering issue between stack_mgr.c and af_packet.c.

>=20
> The problem about the sending messages still happen, i only can send 16=
 messages. Please check if you have the same problem in one of your machi=
nes.
>=20
> In the loopback device i can send several messages (my test send 1000) =
but none is received in the rcv program.=20
> The loopback device works in a different way?

Yeah, loopback device is likely no good idea as it by-passes the stack
manager, see the code. That driver actually needs to be included in the
ETH_P_ALL patch as well, but we could do this later by extracting the
common features of the stack manager loop and the loopback driver into
one common function.

Jan

Re: [RTnet-developers] Test programs for SOCK_RAW and ETH_P_ALL

[Jorge A. <j-a...@cr...>]

Jan,

In attach follows the test cases for the SOCK_RAW and ETH_P_ALL features. Its two files written in C code.
One for sending messages, and other to read they using the ETH_P_ALL feature.

Please test the patch i send to you, after the refactoring that you need to do, and comment.

The problem about the sending messages still happen, i only can send 16 messages. Please check if you have the same problem in one of your machines.

In the loopback device i can send several messages (my test send 1000) but none is received in the rcv program. 
The loopback device works in a different way?

Em Segunda, 11 de Setembro de 2006 18:36, o Jan Kiszka escreveu:
> Jorge Almeida wrote:
> > OK.
> > 
> > I checkout the SVN and made the diff (it's in attach).
> 
> Sigh, your patch reverts a bulk of changes I made over the last weeks.
> This will need some cleanup first.
> 
> What about the test cases? Would be very helpful now to check if things
> work as expected after the refactoring.
> 
> > 
> > The RAW_SOCKET and ETH_P_ALL features are already mixed in the code. I must return to an early stage to generate two different patches.
> > But i send you a pach only for RAW_SOCKET some time ago (28-08-2006). That code didn't change.
> 
> Despite my remarks? Mmh...
> 
> > 
> > 
> > Regarding the other question in the last e-mail, about the queue behaviour, i can only send 16 messages, after that the send function return -105 for error code for every message that i try to send.
> > Error 105 - No Buffer Space Available. 
> > Even using 0x1234 has protocol type to send the messages.
> > But using this type of protocol the messages are only sended to the reading buffer, and not to the writting too.
> 
> Let me check if I understood this correctly: Although there cannot
> arrive any packets to the send-only socket now, it still runs out of
> buffers? Can you reproduce such problem over the loopback device as
> well? I'm asking as the best thing to analyse this issue now is to have
> a free-standing test case.
> 
> Jan
> 
> 

-- 
Jorge Almeida
j-a...@cr...

DISCLAIMER: This message may contain confidential information or privileged material and is intended only for the individual(s) named. If you are not a named addressee and mistakenly received this message you should not copy or otherwise disseminate it: please delete this e-mail from your system and notify the sender immediately. E-mail transmissions are not guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete or contain viruses. Therefore, the sender does not accept liability for any errors or omissions in the contents of this message that arise as a result of e-mail transmissions. Please request a hard copy version if verification is required. Critical Software, SA.

Re: [RTnet-developers] Patches for SOCKET_RAW and ETH_P_ALL

[Jan K. <jan...@we...>]

Hi Jorge,

some review on your latest patch.

Jorge Almeida wrote:
> -----------------------------------------------------------------------=
-
>=20
> Index: stack/include/rtnet_socket.h
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- stack/include/rtnet_socket.h	(revision 1060)
> +++ stack/include/rtnet_socket.h	(working copy)
> @@ -48,14 +48,14 @@
> =20
>      rtdm_lock_t             param_lock;
> =20
> -    unsigned int            priority;
> -    nanosecs_rel_t          timeout;    /* receive timeout, 0 for infi=
nite */
> +    volatile unsigned int   priority;
> +    nanosecs_t              timeout;    /* receive timeout, 0 for infi=
nite */

Please kill those downgrades to a pre-0.9.5 era.

> =20
>      rtdm_sem_t              pending_sem;
>  #ifdef CONFIG_RTNET_RTDM_SELECT
>      wait_queue_primitive_t  *wakeup_select; /* for selecting calls - t=
his
> -                                               SHOULD be the head of a=
 wait
> -                                               queue mechanism. */
> +					       SHOULD be the head of a wait
> +					       queue mechanism. */

Whitespace changes ought to be avoided.

>  #endif /* CONFIG_RTNET_RTDM_SELECT*/
> =20
>      void                    (*callback_func)(struct rtdm_dev_context *=
,
> @@ -78,7 +78,7 @@
>          /* packet socket specific */
>          struct {
>              struct rtpacket_type packet_type;
> -            int                  ifindex;
> +            volatile int         ifindex;

Please update.

>          } packet;
>      } prot;
>  };
> @@ -104,4 +104,7 @@
>                                rtdm_user_info_t *user_info,
>                                int request, void *arg);
> =20
> +#define PACKET_SOCKET "PACKET_SOCKET"
> +#define PACKET_SOCKET_LENGTH 13
> +

Mmh, that's part of a story I do not fully understand yet.

>  #endif  /* __RTNET_SOCKET_H_ */
> Index: stack/include/rtskb.h
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- stack/include/rtskb.h	(revision 1060)
> +++ stack/include/rtskb.h	(working copy)
> @@ -166,12 +166,12 @@
>      struct rtsocket     *sk;        /* assigned socket */
>      struct rtnet_device *rtdev;     /* source or destination device */=

> =20
> -    nanosecs_abs_t      time_stamp; /* arrival or transmission (RTcap)=
 time */
> +    nanosecs_t          time_stamp; /* arrival or transmission (RTcap)=
 time */
> =20
>      /* patch address of the transmission time stamp, can be NULL
>       * calculation: *xmit_stamp =3D cpu_to_be64(time_in_ns + *xmit_sta=
mp)
>       */
> -    nanosecs_abs_t      *xmit_stamp;
> +    nanosecs_t          *xmit_stamp;
> =20
>      /* transport layer */
>      union
> @@ -223,7 +223,7 @@
>      struct rtskb        *cap_next;  /* used for capture queue         =
      */
>      unsigned char       *cap_start; /* start offset for capturing     =
      */
>      unsigned int        cap_len;    /* capture length of this rtskb   =
      */
> -    nanosecs_abs_t      cap_rtmac_stamp; /* RTmac enqueuing time      =
      */
> +    nanosecs_t          cap_rtmac_stamp; /* RTmac enqueuing time      =
      */
>  #endif
>  };
> =20

To be killed.

> @@ -728,6 +728,7 @@
>  extern unsigned int rtskb_pool_shrink_rt(struct rtskb_queue *pool,
>                                           unsigned int rem_rtskbs);
>  extern int rtskb_acquire(struct rtskb *rtskb, struct rtskb_queue *comp=
_pool);
> +extern int rtskb_clone(struct rtskb *rtskb_1,struct rtskb *rtskb_2,str=
uct rtskb_queue *pool);
> =20
>  extern int rtskb_pools_init(void);
>  extern void rtskb_pools_release(void);
> Index: stack/stack_mgr.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- stack/stack_mgr.c	(revision 1060)
> +++ stack/stack_mgr.c	(working copy)
> @@ -27,6 +27,7 @@
>  #include <rtnet_internal.h>
>  #include <rtskb_fifo.h>
>  #include <stack_mgr.h>
> +#include <rtnet_socket.h>

See my comment on the SOCK_RAW patch (fatal layering violation...)

> =20
> =20
>  static unsigned int stack_mgr_prio =3D RTNET_DEF_STACK_PRIORITY;
> @@ -54,15 +55,13 @@
>      int                     ret =3D 0;
>      rtdm_lockctx_t          context;
> =20
> -
> -    if (pt->type =3D=3D htons(ETH_P_ALL))
> +    if (likely(pt->type =3D=3D htons(ETH_P_ALL) && (strncmp(pt->name,P=
ACKET_SOCKET,PACKET_SOCKET_LENGTH) !=3D 0 )))
>          return -EINVAL;

Please explain: what's the idea of this string comparison?

The name field in rtpacket_type is probably something we should rather
kill at this chance - it's only set but doesn't seem to be read anymore
(or never was actually).

> =20
>      INIT_LIST_HEAD(&pt->list_entry);
>      pt->refcount =3D 0;
> =20
>      hash =3D ntohs(pt->type) & RTPACKET_HASH_KEY_MASK;
> -
>      rtdm_lock_get_irqsave(&rt_packets_lock, context);
>      list_add_tail(&pt->list_entry, &rt_packets[hash]);
>      rtdm_lock_put_irqrestore(&rt_packets_lock, context);
> @@ -83,8 +82,7 @@
> =20
> =20
>      RTNET_ASSERT(pt !=3D NULL, return -EINVAL;);
> -
> -    if (pt->type =3D=3D htons(ETH_P_ALL))
> +    if (likely(pt->type =3D=3D htons(ETH_P_ALL) && (strncmp(pt->name,P=
ACKET_SOCKET,PACKET_SOCKET_LENGTH) !=3D 0 )))
>          return -EINVAL;

See above.

> =20
>      rtdm_lock_get_irqsave(&rt_packets_lock, context);
> @@ -119,7 +117,6 @@
>      rtdev_reference(rtdev);
> =20
>      if (unlikely(rtskb_fifo_insert_inirq(&rx.fifo, skb) < 0)) {
> -        rtdm_printk("RTnet: dropping packet in %s()\n", __FUNCTION__);=


Why? Maybe we can create some warn-once, but we need an alarm that the
system was misdesigned (too few buffers or too high load).

>          kfree_rtskb(skb);
>          rtdev_dereference(rtdev);
>      }
> @@ -153,7 +150,6 @@
>      struct rtnet_device     *rtdev;
>      int                     err;
> =20
> -
>      while (rtdm_event_wait(mgr_event) =3D=3D 0)
>          while (1) {
>            next_packet:
> @@ -161,33 +157,46 @@
>              skb =3D __rtskb_fifo_remove(&rx.fifo);
>              if (!skb)
>                  break;
> -
>              rtdev =3D skb->rtdev;
> =20
>              rtcap_report_incoming(skb);
> =20
>              skb->nh.raw =3D skb->data;
> =20
> +            /*hash for the ETH_P_ALL packets*/
> +            hash =3D ETH_P_ALL & RTPACKET_HASH_KEY_MASK;
> +            rtdm_lock_get_irqsave(&rt_packets_lock, context);
> +            /*for cycle that runs over all list entries to check if an=
yone has ETH_P_ALL for protocol type*/
> +            list_for_each_entry(pt_entry, &rt_packets[hash], list_entr=
y)

It's better to keep ETH_P_ALL separated from normal subscribers. If some
protocol matches the hash key of ETH_P_ALL, we but that subscriber
without any needs into this hotpath.

> +            {
> +                if ( likely(ntohs(pt_entry->type) =3D=3D hash &&=20
> +                     (strncmp(pt_entry->name,PACKET_SOCKET,PACKET_SOCK=
ET_LENGTH) =3D=3D 0 )))
> +                {
> +                    pt_entry->refcount++;
> +                    rtdm_lock_put_irqrestore(&rt_packets_lock, context=
);
> +                    err =3D pt_entry->handler(skb, pt_entry);
> +                    rtdm_lock_get_irqsave(&rt_packets_lock, context);
> +                    pt_entry->refcount--;
> +                    rtdm_lock_put_irqrestore(&rt_packets_lock, context=
);

The last lock_put is bogus, remove it.

> +                }
> +            }
> +            rtdm_lock_put_irqrestore(&rt_packets_lock, context);
> +           =20
>              hash =3D ntohs(skb->protocol) & RTPACKET_HASH_KEY_MASK;
> -
>              rtdm_lock_get_irqsave(&rt_packets_lock, context);

Don't drop and reacquire the same lock here. [And even if we go for a
separate list of ETH_P_ALL listeners: we should use the same lock for it
as well.]

> -
> +            /*for cycle that runs over all list entries*/
>              list_for_each_entry(pt_entry, &rt_packets[hash], list_entr=
y)
>                  if (pt_entry->type =3D=3D skb->protocol) {
>                      pt_entry->refcount++;
>                      rtdm_lock_put_irqrestore(&rt_packets_lock, context=
);
> -
>                      err =3D pt_entry->handler(skb, pt_entry);
> -
>                      rtdm_lock_get_irqsave(&rt_packets_lock, context);
>                      pt_entry->refcount--;
>                      rtdm_lock_put_irqrestore(&rt_packets_lock, context=
);
> -
>                      rtdev_dereference(rtdev);
>                      if (!err)
>                          goto next_packet;
>                  }
> -

Please kill all those code layout changes like the above. They exist to
improve the readability.

>              rtdm_lock_put_irqrestore(&rt_packets_lock, context);
> =20
>              /* don't warn if running in promiscuous mode (RTcap...?) *=
/
> Index: stack/rtskb.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- stack/rtskb.c	(revision 1060)
> +++ stack/rtskb.c	(working copy)
> @@ -483,8 +483,34 @@
>      return 0;
>  }
> =20
> +/** Clone a rtskb to another, allocating a rtskb for the copy  */
> +int rtskb_clone(struct rtskb *rtskb_1,struct rtskb *rtskb_2,struct rts=
kb_queue *pool)

Please rename, e.g. rtskb_1 -> rtskb_new, rtskb_2 -> rtskb.

> +{
> +    if(rtskb_1 =3D=3D NULL)
> +        return -ENOMEM;

Mmh, wouldn't it make sense to also allocate the rtskb here and return
the pointer? That's what Linux does as well. The prototype would then
look like this:

struct rtskb *rtskb_clone(struct rtskb *rtskb, struct rtskb_queue *pool)

> =20
> +    rtskb_1->chain_end  =3D rtskb_1;

Done on allocation - unless you manage rtskbs in your own secrete queue
(which you don't).

> +    memcpy(rtskb_1->data, rtskb_2->data, rtskb_2->len );

That looks wrong. The original rtskb may have already forwarded its data
pointer past the MAC header, and the same must happen to the clone. You
have to account for the current data vs. the mac.raw, copy the full
block to the target, and then move the data pointer of the target
accordingly.

> +    rtskb_1->len        =3D rtskb_2->len;
> +    rtskb_1->time_stamp =3D rtskb_2->time_stamp;
> +    rtskb_1->rtdev      =3D rtskb_2->rtdev;
> +    rtskb_1->protocol   =3D rtskb_2->protocol;
> +    rtskb_1->pool       =3D pool;

rtskb_1->pool is already valid and points to pool, see alloc_rtskb.

> +    rtskb_1->pkt_type   =3D rtskb_2->pkt_type;
> +    rtskb_1->priority   =3D rtskb_2->priority;
> +    rtskb_1->ip_summed =3D rtskb_2->ip_summed;
> +    rtskb_1->csum      =3D rtskb_2->csum;
> +   =20
> +    /*copy ethernet stuff*/
> +    rtskb_1->mac.raw    =3D rtskb_1->data;

Likely bogus, see above.

> +    memcpy(rtskb_1->mac.ethernet->h_dest,rtskb_2->mac.ethernet->h_dest=
, ETH_ALEN);
> +    memcpy(rtskb_1->mac.ethernet->h_source,rtskb_2->mac.ethernet->h_so=
urce, ETH_ALEN);

See above.

> +    rtskb_1->mac.ethernet->h_proto =3D rtskb_2->mac.ethernet->h_proto;=


To be really generic, you would also have to clone rtskb.h and .nh -
even if the original rtskb contains garbage in this regard.

> +   =20
> +    return 0;
> +}
> =20
> +
>  int rtskb_pools_init(void)
>  {
>      rtskb_slab_pool =3D kmem_cache_create("rtskb_slab_pool",
> @@ -547,6 +573,7 @@
>  EXPORT_SYMBOL(global_pool);
> =20
>  EXPORT_SYMBOL(rtskb_acquire);
> +EXPORT_SYMBOL(rtskb_clone);
> =20
>  #ifdef CONFIG_RTNET_CHECKED
>  EXPORT_SYMBOL(rtskb_over_panic);
> Index: stack/packet/af_packet.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- stack/packet/af_packet.c	(revision 1060)
> +++ stack/packet/af_packet.c	(working copy)
> @@ -43,31 +43,53 @@
>      void            (*callback_func)(struct rtdm_dev_context *, void *=
);
>      void            *callback_arg;
>      rtdm_lockctx_t  context;
> -
> -
> +    struct rtskb    *clone_skb =3D NULL;
> +    unsigned short  hash =3D 0;
> +    int             err =3D -1;
> +  =20
>      if (unlikely((ifindex !=3D 0) && (ifindex !=3D skb->rtdev->ifindex=
)))
>          return EUNATCH;
> +   =20
> +    /*hash for the ETH_P_ALL packets*/
> +    hash =3D ETH_P_ALL & RTPACKET_HASH_KEY_MASK;
> +    if(likely(ntohs(pt->type) =3D=3D hash &&=20
> +       (strncmp(pt->name,PACKET_SOCKET,PACKET_SOCKET_LENGTH) =3D=3D 0 =
)))
> +    {
> +        clone_skb =3D alloc_rtskb(skb->len,&sock->skb_pool);
> +        if(clone_skb =3D=3D NULL) return -ENOMEM;
> +        rtdm_lock_get_irqsave(&rt_packets_lock, context);
> +        err =3D rtskb_clone(clone_skb, skb, &sock->skb_pool);
> +        rtdm_lock_put_irqrestore(&rt_packets_lock, context);

What's the lock protecting?

> +    }
> +    else
> +    {
> +        err =3D rtskb_acquire(skb, &sock->skb_pool);
> +        clone_skb =3D skb;
> +    }
> =20
> -    if (unlikely(rtskb_acquire(skb, &sock->skb_pool) !=3D 0))
> -        kfree_rtskb(skb);
> -    else {
> -        rtdev_reference(skb->rtdev);
> -        rtskb_queue_tail(&sock->incoming, skb);
> +    if ( err !=3D 0)
> +    {
> +        kfree_rtskb(clone_skb);
> +    }
> +    else=20
> +    {
> +        rtdev_reference(clone_skb->rtdev);
> +        rtskb_queue_tail(&sock->incoming, clone_skb);
> +       =20
>          rtdm_sem_up(&sock->pending_sem);
> =20
>          rtdm_lock_get_irqsave(&sock->param_lock, context);
>          callback_func =3D sock->callback_func;
>          callback_arg  =3D sock->callback_arg;
>          rtdm_lock_put_irqrestore(&sock->param_lock, context);
> -
>          if (callback_func)
> +        {
>              callback_func(rt_socket_context(sock), callback_arg);
> +        }
>      }
>      return 0;
>  }
> =20
> -
> -
>  /***
>   *  rt_packet_bind
>   */
> @@ -89,7 +111,7 @@
> =20
>      rtdm_lock_get_irqsave(&sock->param_lock, context);
> =20
> -    /* release exisiting binding */
> +    /* release existing binding */

That's an unrelated fix I accept. :)

>      if ((pt->type !=3D 0) && ((ret =3D rtdev_remove_pack(pt)) < 0)) {
>          rtdm_lock_put_irqrestore(&sock->param_lock, context);
>          return ret;
> @@ -100,7 +122,7 @@
> =20
>      /* if protocol is non-zero, register the packet type */
>      if (sock->protocol !=3D 0) {
> -        pt->name        =3D "PACKET_SOCKET";
> +        pt->name        =3D PACKET_SOCKET;
>          pt->handler     =3D rt_packet_rcv;
>          pt->err_handler =3D NULL;
> =20
> @@ -169,13 +191,13 @@
> =20
>      if ((ret =3D rt_socket_init(sockctx)) !=3D 0)
>          return ret;
> -
> +   =20
>      sock->prot.packet.packet_type.type =3D protocol;
>      sock->prot.packet.ifindex          =3D 0;
> =20
>      /* if protocol is non-zero, register the packet type */
>      if (protocol !=3D 0) {
> -        sock->prot.packet.packet_type.name        =3D "PACKET_SOCKET";=

> +        sock->prot.packet.packet_type.name        =3D PACKET_SOCKET;
>          sock->prot.packet.packet_type.handler     =3D rt_packet_rcv;
>          sock->prot.packet.packet_type.err_handler =3D NULL;
> =20
> @@ -271,13 +293,12 @@
>      struct ethhdr       *eth;
>      struct sockaddr_ll  *sll;
>      int                 ret;
> -    nanosecs_abs_t      timeout =3D sock->timeout;
> +    nanosecs_t          timeout =3D sock->timeout;
> =20
> -
>      /* non-blocking receive? */
>      if (testbits(msg_flags, MSG_DONTWAIT))
>          timeout =3D -1;
> -
> +   =20
>      ret =3D rtdm_sem_timeddown(&sock->pending_sem, timeout, NULL);
>      if (unlikely(ret < 0)) {
>          if ((ret !=3D -EWOULDBLOCK) && (ret !=3D -ETIMEDOUT))
> @@ -287,9 +308,7 @@
> =20
>      skb =3D rtskb_dequeue_chain(&sock->incoming);
>      RTNET_ASSERT(skb !=3D NULL, return -EFAULT;);
> -
>      eth =3D skb->mac.ethernet;
> -
>      sll =3D msg->msg_name;
> =20
>      /* copy the address */
> @@ -313,10 +332,38 @@
>          copy_len =3D len;
>          msg->msg_flags |=3D MSG_TRUNC;
>      }
> +   =20
> +    if(sockctx->device->socket_type =3D=3D SOCK_DGRAM)
> +    {
> +        /* copy the data */
> +        rt_memcpy_tokerneliovec(msg->msg_iov, skb->data, copy_len);
> +    }
> +    else if (sockctx->device->socket_type =3D=3D SOCK_RAW)
> +    {
> +        int nBufferLen =3D 0;
> +        unsigned char puchBuffer[ETH_FRAME_LEN] =3D {0};
> +           =20
> +        memcpy((puchBuffer+nBufferLen), eth->h_dest,ETH_ALEN);
> +        nBufferLen +=3D ETH_ALEN;
> =20
> -    /* copy the data */
> -    rt_memcpy_tokerneliovec(msg->msg_iov, skb->data, copy_len);
> +        memcpy((puchBuffer+nBufferLen), eth->h_source,ETH_ALEN);
> +        nBufferLen +=3D ETH_ALEN;
> =20
> +        memcpy((puchBuffer+nBufferLen), &(eth->h_proto),2);
> +        nBufferLen +=3D 2;
> +
> +        memcpy((puchBuffer+nBufferLen), skb->data, copy_len);
> +        nBufferLen +=3D copy_len;
> +
> +        /* copy the data to user */  =20
> +        rt_memcpy_tokerneliovec(msg->msg_iov, puchBuffer, nBufferLen);=

> +        /* new buffer length*/
> +        real_len =3D nBufferLen;
> +    }
> +    else
> +    {
> +        rtdm_printk("Packet: Socket type Not Supported\n");
> +    }
>      if ((msg_flags & MSG_PEEK) =3D=3D 0) {
>          rtdev_dereference(skb->rtdev);
>          kfree_rtskb(skb);
> @@ -324,7 +371,6 @@
>          rtskb_queue_head(&sock->incoming, skb);
>          rtdm_sem_up(&sock->pending_sem);
>      }
> -
>      return real_len;
>  }
> =20
> @@ -383,13 +429,34 @@
> =20
>      rtskb->rtdev    =3D rtdev;
>      rtskb->priority =3D sock->priority;
> -
> -    if (rtdev->hard_header) {
> -        ret =3D rtdev->hard_header(rtskb, rtdev, ntohs(sll->sll_protoc=
ol),
> -                                 sll->sll_addr, rtdev->dev_addr, rtskb=
->len);
> -        if (ret < 0)
> -            goto err;
> +    // SOCK_DGRAM
> +    if(sockctx->device->socket_type =3D=3D SOCK_DGRAM)
> +    {
> +        if (rtdev->hard_header) {
> +            ret =3D rtdev->hard_header(rtskb, rtdev, ntohs(sll->sll_pr=
otocol),
> +                                    sll->sll_addr, rtdev->dev_addr, rt=
skb->len);
> +            if (ret < 0)
> +                goto err;
> +        }
>      }
> +    // SOCK_RAW
> +    else if (sockctx->device->socket_type =3D=3D SOCK_RAW)
> +    {
> +        unsigned char puchBuffer[ETH_FRAME_LEN];
> +        if (rtdev->hard_header) {
> +            ret =3D rtdev->hard_header(rtskb, rtdev, (unsigned short)*=
(rtskb->data+ETH_ALEN+ETH_ALEN),
> +                                     sll->sll_addr, (rtskb->data+ETH_A=
LEN), (rtskb->len - ETH_HLEN));
> +            if (ret < 0)
> +                goto err;
> +        }
> +        memcpy(puchBuffer,rtskb->data,rtskb->len);
> +        memcpy(rtskb->data,(puchBuffer+ETH_HLEN),(rtskb->len - ETH_HLE=
N));
> +        rtskb->len =3D rtskb->len - ETH_HLEN;
> +    }
> +    else
> +    {
> +        rtdm_printk("Packet: Socket type Not Supported\n");
> +    }

Regarding all this above: see earlier review.

> =20
>      if ((rtdev->flags & IFF_UP) !=3D 0) {
>          if ((ret =3D rtdev_xmit(rtskb)) =3D=3D 0)
> @@ -409,8 +476,6 @@
>      return ret;
>  }
> =20
> -
> -
>  static struct rtdm_device   packet_proto_dev =3D {
>      struct_version:     RTDM_DEVICE_STRUCT_VER,
> =20
> @@ -430,7 +495,7 @@
>          ioctl_nrt:      rt_packet_ioctl,
>          recvmsg_rt:     rt_packet_recvmsg,
>          sendmsg_rt:     rt_packet_sendmsg
> -    },
> +},
> =20
>      device_class:       RTDM_CLASS_NETWORK,
>      device_sub_class:   RTDM_SUBCLASS_RTNET,
> @@ -443,15 +508,55 @@
>  };
> =20
> =20
> +static struct rtdm_device   raw_packet_proto_dev =3D {
> +    struct_version:     RTDM_DEVICE_STRUCT_VER,
> +
> +    device_flags:       RTDM_PROTOCOL_DEVICE,
> +    context_size:       sizeof(struct rtsocket),
> +
> +    protocol_family:    PF_PACKET,
> +    socket_type:        SOCK_RAW,
> +
> +    socket_rt:          rt_packet_socket,
> +    socket_nrt:         rt_packet_socket,
> +
> +    ops: {
> +        close_rt:       rt_packet_close,
> +        close_nrt:      rt_packet_close,
> +        ioctl_rt:       rt_packet_ioctl,
> +        ioctl_nrt:      rt_packet_ioctl,
> +        recvmsg_rt:     rt_packet_recvmsg,
> +        sendmsg_rt:     rt_packet_sendmsg
> +    },
> +
> +    device_class:       RTDM_CLASS_NETWORK,
> +    device_sub_class:   RTDM_SUBCLASS_RTNET,
> +    driver_name:        "rtpacket",
> +    driver_version:     RTNET_RTDM_VER,
> +    peripheral_name:    "Real-Time Packet Socket Interface",
> +    provider_name:      rtnet_rtdm_provider_name,
> +
> +    proc_name:          "PACKET_RAW"
> +};
> +
> +
>  int __init rt_packet_proto_init(void)
>  {
> -    return rtdm_dev_register(&packet_proto_dev);
> +    int nReturn =3D -1;
> +    nReturn =3D rtdm_dev_register(&packet_proto_dev);
> +    if(nReturn !=3D 0)
> +     rtdm_printk("Error registing DGRAM socket. Return =3D %d\n",nRetu=
rn);
> +    nReturn =3D rtdm_dev_register(&raw_packet_proto_dev);
> +    if(nReturn !=3D 0)
> +        rtdm_printk("Error registing RAW socket. Return =3D %d\n",nRet=
urn);
> +    return 0;

See earlier review.

>  }
> =20
> =20
>  void rt_packet_proto_release(void)
>  {
>      rtdm_dev_unregister(&packet_proto_dev, 1000);
> +    rtdm_dev_unregister(&raw_packet_proto_dev, 1000);
>  }
> =20
> =20

Jan

Re: [RTnet-developers] Patches for SOCKET_RAW and ETH_P_ALL

[Jan K. <jan...@we...>]

Jorge Almeida wrote:
> OK.
>=20
> I checkout the SVN and made the diff (it's in attach).

Sigh, your patch reverts a bulk of changes I made over the last weeks.
This will need some cleanup first.

What about the test cases? Would be very helpful now to check if things
work as expected after the refactoring.

>=20
> The RAW_SOCKET and ETH_P_ALL features are already mixed in the code. I =
must return to an early stage to generate two different patches.
> But i send you a pach only for RAW_SOCKET some time ago (28-08-2006). T=
hat code didn't change.

Despite my remarks? Mmh...

>=20
>=20
> Regarding the other question in the last e-mail, about the queue behavi=
our, i can only send 16 messages, after that the send function return -10=
5 for error code for every message that i try to send.
> Error 105 - No Buffer Space Available.=20
> Even using 0x1234 has protocol type to send the messages.
> But using this type of protocol the messages are only sended to the rea=
ding buffer, and not to the writting too.

Let me check if I understood this correctly: Although there cannot
arrive any packets to the send-only socket now, it still runs out of
buffers? Can you reproduce such problem over the loopback device as
well? I'm asking as the best thing to analyse this issue now is to have
a free-standing test case.

Jan

Re: [RTnet-developers] Patches for SOCKET_RAW and ETH_P_ALL

[Jan K. <jan...@we...>]

Jorge Almeida wrote:
> OPS Sorry :)
>=20
> There it goes :)
>=20
>=20
> JAN,
>=20
> In attach goes the patches for the SOCKET_RAW and ETH_P_ALL functionali=
ties for RTnet.
> The only thing that was not done was the configurable option for the ET=
H_P_ALL.=20
>=20

Thanks, will have a look at them later.

>=20
> After some tests in the same computer, with two different cards, i find=
 something interesting that i need to talk with you (or anybody that that=
 wants to participate in the discution).

Both NICs attached to the same network?

> I have two small test programs, one for sending, and another one to rec=
eive the message. In both there's one socket opened with the ETH_P_ALL fe=
ature. Making two different filedescriptors in the same machine.
> After i send the message it is sended for both sockets (REmenber that w=
e are in the same machine), but i only read from one.=20
> This means, after some time the queue for the opened socket used to wri=
te only (not read) will fill.=20
> Wath is the behaviour for this problem?

Buffers for TX and RX are taken from the same pool. Thus, once incoming
unhandled packets consumed all of them, TX will fail.

>=20
> This queue should fill and start to give errors when no more memory is =
available?
> Or we should not send the message to one sockt thas is only writing? Ho=
w can we know if the socket is only writting?
> Or we can set a maximum value to the queued message in one determined q=
ueue? (I think this is the best option)

Simple solution: if you have a send-only socket, don't open it with
ETH_P_ALL. Use some arbitrary unused protocol instead. This also helps
to reduce the system load due to unnecessary packet delivery via costly
cloning.

Jan

[RTnet-developers] Patches for SOCKET_RAW and ETH_P_ALL

[Jorge A. <j-a...@cr...>]

OPS Sorry :)

There it goes :)


JAN,

In attach goes the patches for the SOCKET_RAW and ETH_P_ALL functionalities for RTnet.
The only thing that was not done was the configurable option for the ETH_P_ALL. 


After some tests in the same computer, with two different cards, i find something interesting that i need to talk with you (or anybody that that wants to participate in the discution).
I have two small test programs, one for sending, and another one to receive the message. In both there's one socket opened with the ETH_P_ALL feature. Making two different filedescriptors in the same machine.
After i send the message it is sended for both sockets (REmenber that we are in the same machine), but i only read from one. 
This means, after some time the queue for the opened socket used to write only (not read) will fill. 
Wath is the behaviour for this problem?

This queue should fill and start to give errors when no more memory is available?
Or we should not send the message to one sockt thas is only writing? How can we know if the socket is only writting?
Or we can set a maximum value to the queued message in one determined queue? (I think this is the best option)

Please comment.

Greetings.

-- 
Jorge Almeida
j-a...@cr...

DISCLAIMER: This message may contain confidential information or privileged material and is intended only for the individual(s) named. If you are not a named addressee and mistakenly received this message you should not copy or otherwise disseminate it: please delete this e-mail from your system and notify the sender immediately. E-mail transmissions are not guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete or contain viruses. Therefore, the sender does not accept liability for any errors or omissions in the contents of this message that arise as a result of e-mail transmissions. Please request a hard copy version if verification is required. Critical Software, SA.

[RTnet-developers] Patches for SOCKET_RAW and ETH_P_ALL

[Jorge A. <j-a...@cr...>]

JAN,

In attach goes the patches for the SOCKET_RAW and ETH_P_ALL functionalities for RTnet.
The only thing that was not done was the configurable option for the ETH_P_ALL. 


After some tests in the same computer, with two different cards, i find something interesting that i need to talk with you (or anybody that that wants to participate in the discution).
I have two small test programs, one for sending, and another one to receive the message. In both there's one socket opened with the ETH_P_ALL feature. Making two different filedescriptors in the same machine.
After i send the message it is sended for both sockets (REmenber that we are in the same machine), but i only read from one. 
This means, after some time the queue for the opened socket used to write only (not read) will fill. 
Wath is the behaviour for this problem?

This queue should fill and start to give errors when no more memory is available?
Or we should not send the message to one sockt thas is only writing? How can we know if the socket is only writting?
Or we can set a maximum value to the queued message in one determined queue? (I think this is the best option)

Please comment.

Greetings.

-- 
Jorge Almeida
j-a...@cr...
DISCLAIMER: This message may contain confidential information or privileged material and is intended only for the individual(s) named. If you are not a named addressee and mistakenly received this message you should not copy or otherwise disseminate it: please delete this e-mail from your system and notify the sender immediately. E-mail transmissions are not guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete or contain viruses. Therefore, the sender does not accept liability for any errors or omissions in the contents of this message that arise as a result of e-mail transmissions. Please request a hard copy version if verification is required. Critical Software, SA.

Re: [RTnet-developers] rtskb_acquire

[Jan K. <jan...@we...>]

Jorge Almeida wrote:
> Em Sexta, 1 de Setembro de 2006 15:00, o Jan Kiszka escreveu:
>> Jorge Almeida wrote:
>>> Hi Jan,
>>>
>>> Just one small question.
>>> When the rtskb are created the memory indicated by the pointers prese=
nt in the struct rtskb are allocated too?
>> In contrast to Linux skbs, rtskbs consist of a single piece: the contr=
ol
>> head + the buffer tail.
>>
>>> For example the rtdev pointer points to one area of allocated memory =
for this rtskb, or this pointer points to a unique area that refers to th=
e rtdev struct of the real device?
>> The buffer pointers (head, tail, end, ...) are initialised to point at=

>> their own buffer. rtdev is an external reference to the unique
>> associated networking device. It's no buffer, it's the control structu=
re
>> of that device!
>=20
> So all the pointers, except rtdev, have allocated memory for each rtskb=
?
>=20
> I make these questions because the rtskb_clone function that i'm doing.=

>=20

All uncommented stuff of struct rtskb in rtskb.h point to the data
region of the respective rtskb.

Jan

Re: [RTnet-developers] rtskb_acquire

[Jorge A. <j-a...@cr...>]

Em Sexta, 1 de Setembro de 2006 15:00, o Jan Kiszka escreveu:
> Jorge Almeida wrote:
> > Hi Jan,
> > 
> > Just one small question.
> > When the rtskb are created the memory indicated by the pointers present in the struct rtskb are allocated too?
> 
> In contrast to Linux skbs, rtskbs consist of a single piece: the control
> head + the buffer tail.
> 
> > 
> > For example the rtdev pointer points to one area of allocated memory for this rtskb, or this pointer points to a unique area that refers to the rtdev struct of the real device?
> 
> The buffer pointers (head, tail, end, ...) are initialised to point at
> their own buffer. rtdev is an external reference to the unique
> associated networking device. It's no buffer, it's the control structure
> of that device!

So all the pointers, except rtdev, have allocated memory for each rtskb?

I make these questions because the rtskb_clone function that i'm doing.

> 
> Jan
> 
> > 
> > Thanks for an answer.
> > 
> > Em Quinta, 31 de Agosto de 2006 17:12, o Jan Kiszka escreveu:
> > ...
> 
> PS: Please don't cite what you do not comment on.
> 
> 

-- 
Jorge Almeida
j-a...@cr...
DISCLAIMER: This message may contain confidential information or privileged material and is intended only for the individual(s) named. If you are not a named addressee and mistakenly received this message you should not copy or otherwise disseminate it: please delete this e-mail from your system and notify the sender immediately. E-mail transmissions are not guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete or contain viruses. Therefore, the sender does not accept liability for any errors or omissions in the contents of this message that arise as a result of e-mail transmissions. Please request a hard copy version if verification is required. Critical Software, SA.

Re: [RTnet-developers] rtskb_acquire

[Jan K. <jan...@we...>]

Jorge Almeida wrote:
> Hi Jan,
>=20
> Just one small question.
> When the rtskb are created the memory indicated by the pointers present=
 in the struct rtskb are allocated too?

In contrast to Linux skbs, rtskbs consist of a single piece: the control
head + the buffer tail.

>=20
> For example the rtdev pointer points to one area of allocated memory fo=
r this rtskb, or this pointer points to a unique area that refers to the =
rtdev struct of the real device?

The buffer pointers (head, tail, end, ...) are initialised to point at
their own buffer. rtdev is an external reference to the unique
associated networking device. It's no buffer, it's the control structure
of that device!

Jan

>=20
> Thanks for an answer.
>=20
> Em Quinta, 31 de Agosto de 2006 17:12, o Jan Kiszka escreveu:
> ...

PS: Please don't cite what you do not comment on.

Re: [RTnet-developers] rtskb_acquire

[Jorge A. <j-a...@cr...>]

Hi Jan,

Just one small question.
When the rtskb are created the memory indicated by the pointers present in the struct rtskb are allocated too?

For example the rtdev pointer points to one area of allocated memory for this rtskb, or this pointer points to a unique area that refers to the rtdev struct of the real device?

Thanks for an answer.

Em Quinta, 31 de Agosto de 2006 17:12, o Jan Kiszka escreveu:
> Hi Jorge,
> 
> Jorge Almeida wrote:
> > Hello Jan.
> > 
> > Can you please give me a description of wath is done in the rtskb_acquire function in the rtskb.c file line 463.
> 
> That function passes the ownership of a rtskb from rtskb->pool to
> comp_pool (compensation pool). For this purpose, an empty rtskb is
> dequeued from comp_pool and enqueued in rtskb->pool, then rtskb->pool is
> set to comp_pool.
> 
> > 
> > The rtnet is crashing with an SIG=11 in this function when i pass the cloned rtskb to the ETH_P_ALL socket.
> 
> Where precisely? Already considered to install kgdb for such development?
> 
> > 
> > My clone function is the following:
> > /** Clone a rtskb to another, allocating a rtskb for the copy  */
> > int rtskb_clone(struct rtskb *rtskb_1,struct rtskb *rtskb_2,struct rtskb_queue *pool)
> > {
> > 
> >     rtskb_1 = alloc_rtskb(rtskb_2->len,pool);
> >     if(rtskb_1 == NULL)
> >         return -ENOMEM;
> >     
> >     
> >     memcpy(rtskb_1->data, rtskb_2->data, rtskb_2->len );
> >     rtskb_1->len        = rtskb_2->len;
> >     rtskb_1->time_stamp = rtskb_2->time_stamp;
> >     rtskb_1->rtdev      = rtskb_2->rtdev;
> >     rtskb_1->protocol   = rtskb_2->protocol;
> 
> What about the things that happen in rt_eth_type_trans (except for
> rtcap_mark_incoming)? Should case the crash above, but will cause
> troubles when processing the rtskb.
> 
> >     
> >     return 0;
> > }
> > 
> > I check the path between the driver and the stack manager and only find that these fields are necessary.
> > 
> > the task manager does the following for the clone rtskb:
> > 
> >             list_for_each_entry(pt_entry, &rt_packets[hash], list_entry)
> >             {
> >                 rt_printk("List entry protocol= %d %d name = %d\n",
> >                           ntohs(pt_entry->type),hash,strncmp(pt_entry->name,"PACKET_SOCKET",13));
> >                 //the "PACKET_SOCKET" string is defined in line 188 of file af_packet.c
> >                 //this string should be replaced by a #define declaration
> >                 if ( likely(ntohs(pt_entry->type) == hash && 
> >                             (strncmp(pt_entry->name,"PACKET_SOCKET",13) == 0 )))
> >                 {
> >                     rtdm_lock_put_irqrestore(&rt_packets_lock, context);
> 
> Don't drop the lock here when taking it two lines later again.
> 
> > //                     rt_printk("List entry 1 \n");
> >                     
> >                     sock   = container_of(pt_entry, struct rtsocket,
> >                                            prot.packet.packet_type);
> 
> Fatal layering violation! This is a hack, the actual cloning should
> rather take place inside the invoked handler. Same is true for the
> enqueuing. This way you could also avoid to acquire an rtskb that was
> already taken from the socket queue (redundant work).
> 
> >                     
> >                     rtdm_lock_get_irqsave(&rt_packets_lock, context);
> >                     err = rtskb_clone(clone_skb, skb, &sock->skb_pool);
> >                     rtdm_lock_put_irqrestore(&rt_packets_lock, context);
> >                     if(err != 0)
> >                     {
> >                         break;
> >                     }
> >                         
> >                     rtskb_queue_head(&sock->skb_pool,clone_skb);
> >                     //queue the message to fifo head here
> >                     
> >                     
> >                     rtdm_lock_get_irqsave(&rt_packets_lock, context);
> >                     pt_entry->refcount++;
> >                     rtdm_lock_put_irqrestore(&rt_packets_lock, context);
> > //                         rt_printk("List entry 2 \n");
> >                     err = pt_entry->handler(clone_skb, pt_entry);
> > //                         rt_printk("List entry 3 \n");
> >                     rtdm_lock_get_irqsave(&rt_packets_lock, context);
> >                     pt_entry->refcount--;
> >                     rtdm_lock_put_irqrestore(&rt_packets_lock, context);
> > 
> >                 }
> >             }
> > 
> > 
> > I'm not sure if the cloned rtskb shoul be inserted in the queue head or not.
> > 
> 
> Move that stuff to af_packet.c.
> 
> Jan
> 
> 

-- 
Jorge Almeida
j-a...@cr...
DISCLAIMER: This message may contain confidential information or privileged material and is intended only for the individual(s) named. If you are not a named addressee and mistakenly received this message you should not copy or otherwise disseminate it: please delete this e-mail from your system and notify the sender immediately. E-mail transmissions are not guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete or contain viruses. Therefore, the sender does not accept liability for any errors or omissions in the contents of this message that arise as a result of e-mail transmissions. Please request a hard copy version if verification is required. Critical Software, SA.

Re: [RTnet-developers] rtskb_acquire

[Jan K. <jan...@we...>]

Hi Jorge,

Jorge Almeida wrote:
> Hello Jan.
>=20
> Can you please give me a description of wath is done in the rtskb_acqui=
re function in the rtskb.c file line 463.

That function passes the ownership of a rtskb from rtskb->pool to
comp_pool (compensation pool). For this purpose, an empty rtskb is
dequeued from comp_pool and enqueued in rtskb->pool, then rtskb->pool is
set to comp_pool.

>=20
> The rtnet is crashing with an SIG=3D11 in this function when i pass the=
 cloned rtskb to the ETH_P_ALL socket.

Where precisely? Already considered to install kgdb for such development?=


>=20
> My clone function is the following:
> /** Clone a rtskb to another, allocating a rtskb for the copy  */
> int rtskb_clone(struct rtskb *rtskb_1,struct rtskb *rtskb_2,struct rtsk=
b_queue *pool)
> {
>=20
>     rtskb_1 =3D alloc_rtskb(rtskb_2->len,pool);
>     if(rtskb_1 =3D=3D NULL)
>         return -ENOMEM;
>    =20
>    =20
>     memcpy(rtskb_1->data, rtskb_2->data, rtskb_2->len );
>     rtskb_1->len        =3D rtskb_2->len;
>     rtskb_1->time_stamp =3D rtskb_2->time_stamp;
>     rtskb_1->rtdev      =3D rtskb_2->rtdev;
>     rtskb_1->protocol   =3D rtskb_2->protocol;

What about the things that happen in rt_eth_type_trans (except for
rtcap_mark_incoming)? Should case the crash above, but will cause
troubles when processing the rtskb.

>    =20
>     return 0;
> }
>=20
> I check the path between the driver and the stack manager and only find=
 that these fields are necessary.
>=20
> the task manager does the following for the clone rtskb:
>=20
>             list_for_each_entry(pt_entry, &rt_packets[hash], list_entry=
)
>             {
>                 rt_printk("List entry protocol=3D %d %d name =3D %d\n",=

>                           ntohs(pt_entry->type),hash,strncmp(pt_entry->=
name,"PACKET_SOCKET",13));
>                 //the "PACKET_SOCKET" string is defined in line 188 of =
file af_packet.c
>                 //this string should be replaced by a #define declarati=
on
>                 if ( likely(ntohs(pt_entry->type) =3D=3D hash &&=20
>                             (strncmp(pt_entry->name,"PACKET_SOCKET",13)=
 =3D=3D 0 )))
>                 {
>                     rtdm_lock_put_irqrestore(&rt_packets_lock, context)=
;

Don't drop the lock here when taking it two lines later again.

> //                     rt_printk("List entry 1 \n");
>                    =20
>                     sock   =3D container_of(pt_entry, struct rtsocket,
>                                            prot.packet.packet_type);

Fatal layering violation! This is a hack, the actual cloning should
rather take place inside the invoked handler. Same is true for the
enqueuing. This way you could also avoid to acquire an rtskb that was
already taken from the socket queue (redundant work).

>                    =20
>                     rtdm_lock_get_irqsave(&rt_packets_lock, context);
>                     err =3D rtskb_clone(clone_skb, skb, &sock->skb_pool=
);
>                     rtdm_lock_put_irqrestore(&rt_packets_lock, context)=
;
>                     if(err !=3D 0)
>                     {
>                         break;
>                     }
>                        =20
>                     rtskb_queue_head(&sock->skb_pool,clone_skb);
>                     //queue the message to fifo head here
>                    =20
>                    =20
>                     rtdm_lock_get_irqsave(&rt_packets_lock, context);
>                     pt_entry->refcount++;
>                     rtdm_lock_put_irqrestore(&rt_packets_lock, context)=
;
> //                         rt_printk("List entry 2 \n");
>                     err =3D pt_entry->handler(clone_skb, pt_entry);
> //                         rt_printk("List entry 3 \n");
>                     rtdm_lock_get_irqsave(&rt_packets_lock, context);
>                     pt_entry->refcount--;
>                     rtdm_lock_put_irqrestore(&rt_packets_lock, context)=
;
>=20
>                 }
>             }
>=20
>=20
> I'm not sure if the cloned rtskb shoul be inserted in the queue head or=
 not.
>=20

Move that stuff to af_packet.c.

Jan

[RTnet-developers] rtskb_acquire

[Jorge A. <j-a...@cr...>]

Hello Jan.

Can you please give me a description of wath is done in the rtskb_acquire function in the rtskb.c file line 463.

The rtnet is crashing with an SIG=11 in this function when i pass the cloned rtskb to the ETH_P_ALL socket.

My clone function is the following:
/** Clone a rtskb to another, allocating a rtskb for the copy  */
int rtskb_clone(struct rtskb *rtskb_1,struct rtskb *rtskb_2,struct rtskb_queue *pool)
{

    rtskb_1 = alloc_rtskb(rtskb_2->len,pool);
    if(rtskb_1 == NULL)
        return -ENOMEM;
    
    
    memcpy(rtskb_1->data, rtskb_2->data, rtskb_2->len );
    rtskb_1->len        = rtskb_2->len;
    rtskb_1->time_stamp = rtskb_2->time_stamp;
    rtskb_1->rtdev      = rtskb_2->rtdev;
    rtskb_1->protocol   = rtskb_2->protocol;
    
    return 0;
}

I check the path between the driver and the stack manager and only find that these fields are necessary.

the task manager does the following for the clone rtskb:

            list_for_each_entry(pt_entry, &rt_packets[hash], list_entry)
            {
                rt_printk("List entry protocol= %d %d name = %d\n",
                          ntohs(pt_entry->type),hash,strncmp(pt_entry->name,"PACKET_SOCKET",13));
                //the "PACKET_SOCKET" string is defined in line 188 of file af_packet.c
                //this string should be replaced by a #define declaration
                if ( likely(ntohs(pt_entry->type) == hash && 
                            (strncmp(pt_entry->name,"PACKET_SOCKET",13) == 0 )))
                {
                    rtdm_lock_put_irqrestore(&rt_packets_lock, context);
//                     rt_printk("List entry 1 \n");
                    
                    sock   = container_of(pt_entry, struct rtsocket,
                                           prot.packet.packet_type);
                    
                    rtdm_lock_get_irqsave(&rt_packets_lock, context);
                    err = rtskb_clone(clone_skb, skb, &sock->skb_pool);
                    rtdm_lock_put_irqrestore(&rt_packets_lock, context);
                    if(err != 0)
                    {
                        break;
                    }
                        
                    rtskb_queue_head(&sock->skb_pool,clone_skb);
                    //queue the message to fifo head here
                    
                    
                    rtdm_lock_get_irqsave(&rt_packets_lock, context);
                    pt_entry->refcount++;
                    rtdm_lock_put_irqrestore(&rt_packets_lock, context);
//                         rt_printk("List entry 2 \n");
                    err = pt_entry->handler(clone_skb, pt_entry);
//                         rt_printk("List entry 3 \n");
                    rtdm_lock_get_irqsave(&rt_packets_lock, context);
                    pt_entry->refcount--;
                    rtdm_lock_put_irqrestore(&rt_packets_lock, context);

                }
            }


I'm not sure if the cloned rtskb shoul be inserted in the queue head or not.


thanks
-- 
Jorge Almeida
j-a...@cr...
DISCLAIMER: This message may contain confidential information or privileged material and is intended only for the individual(s) named. If you are not a named addressee and mistakenly received this message you should not copy or otherwise disseminate it: please delete this e-mail from your system and notify the sender immediately. E-mail transmissions are not guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete or contain viruses. Therefore, the sender does not accept liability for any errors or omissions in the contents of this message that arise as a result of e-mail transmissions. Please request a hard copy version if verification is required. Critical Software, SA.

Re: [RTnet-developers] Raw Socket

[Jorge A. <j-a...@cr...>]

OK Jan.

I will try to make the B solution to work and if any doubt i ask in the list.

Regarding the SSH frames i don't need they in the socket (I think !!!).



Em Quarta, 30 de Agosto de 2006 17:48, o Jan Kiszka escreveu:
> [put the list on CC again]
> 
> Jorge Almeida wrote:
> > For my application i need the RAW socket catching all the messages in the network, and then they are treated in the rest of the application.
> > 
> > But i have a smaller application, a watchdog, that have a UDP socket for remote control of the first application. 
> > And one of the principles is that they must stay independent from each other, i mean without communication.
> > 
> > And not very often i yould like to connect to the machine using a ssh client for log collector or maybe a maintenance operation for example.
> 
> SSH would either be tunnelled (when using a closed, RTmac-managed
> network) or be forwarded to the rtnetproxy (when installed). But do you
> need those packets on the ETH_P_ALL socket as well?
> 
> > 
> > For that to work i think the A solution is not enough (!!!????)
> 
> Do you need ANY incoming packet delivered to more than one socket? If
> yes, A is a no-go. :(
> 
> > 
> > But maybe the B 1 solution, with copy, is enough.
> 
> Ok, let's go for this and optimise later (shared rtskbs).
> 
> > 
> > But i stuck in the rtskb copy, because i dont know all the architectur and concept of the rtskb.
> > If anyone could help me with the rtskb_copy function.
> > 
> > I've started with this:
> > alloc a rtskb from the rtskb_queue (this is the queue from the socket that gonna red the message after) so this rtskb will be release shortly.
> > But now i dont know if a can make a simple memcpy copying the all structure at once or i need to make some more complicated kind of copy.
> > After the copy the result rtskb will be inserted in the head of the driver receiving queue and willbe acquired by the socket function to the socket queue as normaly, 
> > with this the same function is used for ETH_P_ALL or any other protocol.
> > 
> > // int rtskb_copy(struct rtskb *rtskb_1,struct rtskb *rtskb_2,struct rtskb_queue *pool)
> > // {
> > // 
> > //     rtskb_1 = alloc_rtskb(rtskb_2->len,pool);
> > //     if(rtskb_1 = NULL)
> > //         return -ENOMEM;
> > //     
> > //     
> > // 
> > // 
> > // }
> > 
> 
> memcpy the data, but not the control structure. I would suggest to
> carefully step through the short life of an rtskb from its allocation in
> the driver's rx handler up to the point where you want to clone it (BTW,
> "clone" is the better term for the future, copy is just the current way
> to achieve this). What fields are touched? What do they express?
> =>Translate. If you stumble of some fields of an rtskb that you don't
> understand, come back and ask here.
> 
> Jan
> 
> 

-- 
Jorge Almeida
j-a...@cr...
DISCLAIMER: This message may contain confidential information or privileged material and is intended only for the individual(s) named. If you are not a named addressee and mistakenly received this message you should not copy or otherwise disseminate it: please delete this e-mail from your system and notify the sender immediately. E-mail transmissions are not guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete or contain viruses. Therefore, the sender does not accept liability for any errors or omissions in the contents of this message that arise as a result of e-mail transmissions. Please request a hard copy version if verification is required. Critical Software, SA.

Re: [RTnet-developers] Raw Socket

[Jan K. <jan...@we...>]

[put the list on CC again]

Jorge Almeida wrote:
> For my application i need the RAW socket catching all the messages in t=
he network, and then they are treated in the rest of the application.
>=20
> But i have a smaller application, a watchdog, that have a UDP socket fo=
r remote control of the first application.=20
> And one of the principles is that they must stay independent from each =
other, i mean without communication.
>=20
> And not very often i yould like to connect to the machine using a ssh c=
lient for log collector or maybe a maintenance operation for example.

SSH would either be tunnelled (when using a closed, RTmac-managed
network) or be forwarded to the rtnetproxy (when installed). But do you
need those packets on the ETH_P_ALL socket as well?

>=20
> For that to work i think the A solution is not enough (!!!????)

Do you need ANY incoming packet delivered to more than one socket? If
yes, A is a no-go. :(

>=20
> But maybe the B 1 solution, with copy, is enough.

Ok, let's go for this and optimise later (shared rtskbs).

>=20
> But i stuck in the rtskb copy, because i dont know all the architectur =
and concept of the rtskb.
> If anyone could help me with the rtskb_copy function.
>=20
> I've started with this:
> alloc a rtskb from the rtskb_queue (this is the queue from the socket t=
hat gonna red the message after) so this rtskb will be release shortly.
> But now i dont know if a can make a simple memcpy copying the all struc=
ture at once or i need to make some more complicated kind of copy.
> After the copy the result rtskb will be inserted in the head of the dri=
ver receiving queue and willbe acquired by the socket function to the soc=
ket queue as normaly,=20
> with this the same function is used for ETH_P_ALL or any other protocol=
=2E
>=20
> // int rtskb_copy(struct rtskb *rtskb_1,struct rtskb *rtskb_2,struct rt=
skb_queue *pool)
> // {
> //=20
> //     rtskb_1 =3D alloc_rtskb(rtskb_2->len,pool);
> //     if(rtskb_1 =3D NULL)
> //         return -ENOMEM;
> //    =20
> //    =20
> //=20
> //=20
> // }
>=20

memcpy the data, but not the control structure. I would suggest to
carefully step through the short life of an rtskb from its allocation in
the driver's rx handler up to the point where you want to clone it (BTW,
"clone" is the better term for the future, copy is just the current way
to achieve this). What fields are touched? What do they express?
=3D>Translate. If you stumble of some fields of an rtskb that you don't
understand, come back and ask here.

Jan

Re: [RTnet-developers] Raw Socket

[Jan K. <jan...@we...>]

Jorge Almeida wrote:
> Hello Jan,
> Sorry for the late answer but our mail server is not working very well =
and i don get your e-mail. I see it in the list archives.

Fortunately, I didn't get bored in the meantime. ;)

>=20
> I have a small test program in C++ for the RAW SOCKET but i don have it=
 prepared for publication. This week i will make a simple one and send to=
 the list/you.

That's fine.

>=20
>=20
> Regarding the ETH_P_ALL issue:
>=20
> I've reached a point that maybe is the solution for the ETH_P_ALL issue=
, but maybe is not the best solution:
>=20
> In the stack manager task, where the rtskb is sended to the correct soc=
ket, defined with some kind of protocol, the rtskb is requeued for anothe=
r queue, this is, the rtskb from the driver queue is removed and sended f=
or the socket queue (right??).

Right. In exchange, the driver RX queue receives an empty skb from the
target socket. If the socket has none, the packet is dropped (and the
skb is recycled). That's the strict resource management I'm often
referring to.

> Like ETH_P_ALL is for any kind of protocol, every rtskb must be sended =
for the socket that defines the ETH_P_ALL  protocol and, with the corrent=
 implementation, one rtskb only is able to be queued in one socket queue,=
 and only one. This is a problem if any socket is ETH_P_ALL.=20
> My ideia is to make a cycle where all the open sockets are checked for =
ETH_P_ALL  protocol, and then pass the rtskb to them, but theres a proble=
m with this method, i need to make a copy of the rtskb, and is in that po=
int that i've stucked now.=20
> (Is there any function that makes a rtskb copy? How can i make a rtskb =
copy?)
>=20
> The ideia after the copy is to head queue the copyed rtskb in the drive=
r queue and process it in the regulat way by the rt_packet_rcv function i=
n af_packet.c file, for the socket with ETH_P_ALL  flag.
>=20
> This ideia has one weak point. If theres a lot of ETH_P_ALL sockets the=
 copy time for all copies can be very exaustive (in real-time contex) but=
 the number of ETH_P_ALL sockets, in the major number of applications, is=
 very low, the most times only one in each machine, working like a networ=
k sniffer with a dispatcher. And the time for one rtskb copy is not very =
important (I think). So maybe this solution is the best, because we dont =
miss any message in the ETH_P_ALL  sockets neither in the other kind of s=
ockets.
>=20
> If anyone has a better idea please explain it now. I'm implementing the=
 ETH_P_ALL  feauture at the moment and i only have until next friday (my =
boss said :) ).

Well, I would say we should go for two flavours of ETH_P_ALL (and maybe
stop after implementing the first one :)): Variant A only delivers those
packets to a *single* ETH_P_ALL listener if no one else cared for the
protocol when consulting the rt_packets[] table ("fall-through" as
sketched in earlier mails). Variant B would require some rtskb cloning
(either dumb copying or smarter but more complex sharing) and would then
be able to provide "real" ETH_P_ALL for even more than only one listener.=


A is more something to finish till next Friday, B with copying maybe as
well (but rtskb_copy does not exist yet). B with rtskb sharing is
definitely a larger project. I have some ideas on rtskb sharing for
quite a while (also interesting for potential multicast support), but as
they include a refactoring of the sensitive rtskb subsystem, such a
thing cannot be done quickly.

So my question to you: Would variant A already suffice your project's
requirements?

> =20
> Bellow follows an excert of the new for cycle in stack_mgr thread for t=
he ETH_P_ALL feature.
>=20
> This is an excert of the new stack_mgr.c
>=20
>             rtcap_report_incoming(skb);
>=20
>             skb->nh.raw =3D skb->data;
> /* start new for cycle*/       =20
> //hash for the ETH_P_ALL packets
>             hash =3D ETH_P_ALL & RTPACKET_HASH_KEY_MASK;
>             rtdm_lock_get_irqsave(&rt_packets_lock, context);
>                 rt_printk("Before 1st List: hash =3D %d\n",hash);
> //for cycle that runs over all list entries to check if anyone has ETH_=
P_ALL for protocol type
>             list_for_each_entry(pt_entry, &rt_packets[hash], list_entry=
)
>             {   =20
>                 rt_printk("List entry protocol=3D %d %d name =3D %d\n",=
ntohs(pt_entry->type),hash,strncmp(pt_entry->name,"PACKET_SOCKET",13));
>                 //the "PACKET_SOCKET" string is defined in line 188 of =
file af_packet.c
>                 //this string should be replaced by a #define declarati=
on
>                 if ( likely(ntohs(pt_entry->type) =3D=3D hash &&=20
>                             (strncmp(pt_entry->name,"PACKET_SOCKET",13)=
 =3D=3D 0 )))
>                 {
>                         rt_printk("List entry 1 \n");
>                     pt_entry->refcount++;
>                     rtdm_lock_put_irqrestore(&rt_packets_lock, context)=
;
>                         rt_printk("List entry 2 \n");
>                     err =3D pt_entry->handler(skb, pt_entry);
>                         rt_printk("List entry 3 \n");
>                     rtdm_lock_get_irqsave(&rt_packets_lock, context);
>                     pt_entry->refcount--;
>                     rtdm_lock_put_irqrestore(&rt_packets_lock, context)=
;
>=20
>                 }
>             }
>             rtdm_lock_put_irqrestore(&rt_packets_lock, context);
> /* end new for cycle*/       =20
> rt_printk("Before 2nd List \n");
>             hash =3D ntohs(skb->protocol) & RTPACKET_HASH_KEY_MASK;
>             rtdm_lock_get_irqsave(&rt_packets_lock, context);
>             //for cycle that runs over all list entries
>             list_for_each_entry(pt_entry, &rt_packets[hash], list_entry=
)
>                 if (pt_entry->type =3D=3D skb->protocol) {
>=20
>=20
> Greetings
>=20
>=20

General remark on the code: If you really have to extend to fast path of
all receivers, please make such feature configurable (something like
CONFIG_RTNET_ETH_P_ALL). But I hope we can get away with A which will
likely be acceptable even without an extra switch.

Jan

[RTnet-developers] Raw Socket

[Jorge A. <j-a...@cr...>]

Hello Jan,
Sorry for the late answer but our mail server is not working very well and i don get your e-mail. I see it in the list archives.

I have a small test program in C++ for the RAW SOCKET but i don have it prepared for publication. This week i will make a simple one and send to the list/you.


Regarding the ETH_P_ALL issue:

I've reached a point that maybe is the solution for the ETH_P_ALL issue, but maybe is not the best solution:

In the stack manager task, where the rtskb is sended to the correct socket, defined with some kind of protocol, the rtskb is requeued for another queue, this is, the rtskb from the driver queue is removed and sended for the socket queue (right??).
Like ETH_P_ALL is for any kind of protocol, every rtskb must be sended for the socket that defines the ETH_P_ALL  protocol and, with the corrent implementation, one rtskb only is able to be queued in one socket queue, and only one. This is a problem if any socket is ETH_P_ALL. 
My ideia is to make a cycle where all the open sockets are checked for ETH_P_ALL  protocol, and then pass the rtskb to them, but theres a problem with this method, i need to make a copy of the rtskb, and is in that point that i've stucked now. 
(Is there any function that makes a rtskb copy? How can i make a rtskb copy?)

The ideia after the copy is to head queue the copyed rtskb in the driver queue and process it in the regulat way by the rt_packet_rcv function in af_packet.c file, for the socket with ETH_P_ALL  flag.

This ideia has one weak point. If theres a lot of ETH_P_ALL sockets the copy time for all copies can be very exaustive (in real-time contex) but the number of ETH_P_ALL sockets, in the major number of applications, is very low, the most times only one in each machine, working like a network sniffer with a dispatcher. And the time for one rtskb copy is not very important (I think). So maybe this solution is the best, because we dont miss any message in the ETH_P_ALL  sockets neither in the other kind of sockets.

If anyone has a better idea please explain it now. I'm implementing the ETH_P_ALL  feauture at the moment and i only have until next friday (my boss said :) ).
 
Bellow follows an excert of the new for cycle in stack_mgr thread for the ETH_P_ALL feature.

This is an excert of the new stack_mgr.c

            rtcap_report_incoming(skb);

            skb->nh.raw = skb->data;
/* start new for cycle*/        
//hash for the ETH_P_ALL packets
            hash = ETH_P_ALL & RTPACKET_HASH_KEY_MASK;
            rtdm_lock_get_irqsave(&rt_packets_lock, context);
                rt_printk("Before 1st List: hash = %d\n",hash);
//for cycle that runs over all list entries to check if anyone has ETH_P_ALL for protocol type
            list_for_each_entry(pt_entry, &rt_packets[hash], list_entry)
            {    
                rt_printk("List entry protocol= %d %d name = %d\n",ntohs(pt_entry->type),hash,strncmp(pt_entry->name,"PACKET_SOCKET",13));
                //the "PACKET_SOCKET" string is defined in line 188 of file af_packet.c
                //this string should be replaced by a #define declaration
                if ( likely(ntohs(pt_entry->type) == hash && 
                            (strncmp(pt_entry->name,"PACKET_SOCKET",13) == 0 )))
                {
                        rt_printk("List entry 1 \n");
                    pt_entry->refcount++;
                    rtdm_lock_put_irqrestore(&rt_packets_lock, context);
                        rt_printk("List entry 2 \n");
                    err = pt_entry->handler(skb, pt_entry);
                        rt_printk("List entry 3 \n");
                    rtdm_lock_get_irqsave(&rt_packets_lock, context);
                    pt_entry->refcount--;
                    rtdm_lock_put_irqrestore(&rt_packets_lock, context);

                }
            }
            rtdm_lock_put_irqrestore(&rt_packets_lock, context);
/* end new for cycle*/        
rt_printk("Before 2nd List \n");
            hash = ntohs(skb->protocol) & RTPACKET_HASH_KEY_MASK;
            rtdm_lock_get_irqsave(&rt_packets_lock, context);
            //for cycle that runs over all list entries
            list_for_each_entry(pt_entry, &rt_packets[hash], list_entry)
                if (pt_entry->type == skb->protocol) {


Greetings


-- 
Jorge Almeida
j-a...@cr...
DISCLAIMER: This message may contain confidential information or privileged material and is intended only for the individual(s) named. If you are not a named addressee and mistakenly received this message you should not copy or otherwise disseminate it: please delete this e-mail from your system and notify the sender immediately. E-mail transmissions are not guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete or contain viruses. Therefore, the sender does not accept liability for any errors or omissions in the contents of this message that arise as a result of e-mail transmissions. Please request a hard copy version if verification is required. Critical Software, SA.

Re: [RTnet-developers] SOCKET RAW

[Jan K. <jan...@we...>]

Jorge Almeida wrote:
> Jan,
>=20
> In attach i send you a diff file regarding the af_packet.c file where t=
he RAW=20
> SOCKET is implemented, similar to the DGRAM SOCKET.

Two general remarks, rest inlined below:

o The diff parameter "-p" makes the output more readable by adding the
  context of the change.

o Please avoid // comments (maybe except for temporary debug stuff)

>=20
> I've already run some tests on it and everything looks OK.
>=20
> Please run some more, or ask to (to some other interested person) and p=
ublish=20
> the results.
>=20

Do you have some test program? May an enhanced version of the existing
packet-socket demo with selectable mode? Would make it easier to start.

>=20
> Now i'm starting with the ETH_P_ALL to the protocol definition.
> Were, in the code, can i start to see the protocol filter implementatio=
n?
> And wath is the best thing to do regarding ETH_P_ALL? Remove from QUEUE=
 or=20
> copy the message from buffer and it still remains there? The second has=
 the=20
> problem of the queue grows undefined.=20
> Any suggestions?

See my earlier post:

http://www.mail-archive.com/rtnet-users%40lists.sourceforge.net/msg01279.=
html

This would make ETH_P_ALL not true ALL, but it would also not blow up
the current strict resource management scheme. Unless you are building a
packet sniffer, this should be fine for most use cases (and there is
already a sniffer interface).

>=20
>=20
> Thanks
>=20
>=20
>=20
> -----------------------------------------------------------------------=
-
>=20
> --- rtnet-0.9.4/stack/packet/af_packet.c	2006-08-06 19:23:36.000000000 =
+0100
> +++ rtnet-0.9.4/stack/packet/af_packet.c	2006-08-28 16:07:35.000000000 =
+0100
> @@ -89,7 +89,7 @@
> =20
>      rtdm_lock_get_irqsave(&sock->param_lock, context);
> =20
> -    /* release exisiting binding */
> +    /* release existing binding */
>      if ((pt->type !=3D 0) && ((ret =3D rtdev_remove_pack(pt)) < 0)) {
>          rtdm_lock_put_irqrestore(&sock->param_lock, context);
>          return ret;
> @@ -277,7 +277,7 @@
>      /* non-blocking receive? */
>      if (testbits(msg_flags, MSG_DONTWAIT))
>          timeout =3D -1;
> -
> +//     rt_printk("Packet:DEBUG: timeout %lld \n",timeout);
>      ret =3D rtdm_sem_timeddown(&sock->pending_sem, timeout, NULL);
>      if (unlikely(ret < 0)) {
>          if ((ret !=3D -EWOULDBLOCK) && (ret !=3D -ETIMEDOUT))
> @@ -313,10 +313,45 @@
>          copy_len =3D len;
>          msg->msg_flags |=3D MSG_TRUNC;
>      }
> +   =20
> +    if(sockctx->device->socket_type =3D=3D SOCK_DGRAM)
> +    {
> +//         rt_printk("Packet:DEBUG: SOCKET DGRAM SELECTED\n");
> +        /* copy the data */
> +        rt_memcpy_tokerneliovec(msg->msg_iov, skb->data, copy_len);
> +    }
> +    else if (sockctx->device->socket_type =3D=3D SOCK_RAW)
> +    {
> +        int nBufferLen =3D 0;
> +        int nI =3D 0;
> +        unsigned char puchBuffer[ETH_FRAME_LEN] =3D {0};
> +           =20
> +        memcpy((puchBuffer+nBufferLen), eth->h_dest,ETH_ALEN);
> +        nBufferLen +=3D ETH_ALEN;
> =20
> -    /* copy the data */
> -    rt_memcpy_tokerneliovec(msg->msg_iov, skb->data, copy_len);
> +        memcpy((puchBuffer+nBufferLen), eth->h_source,ETH_ALEN);
> +        nBufferLen +=3D ETH_ALEN;
> +
> +        memcpy((puchBuffer+nBufferLen), &(eth->h_proto),2);
> +        nBufferLen +=3D 2;
> =20
> +        memcpy((puchBuffer+nBufferLen), skb->data, copy_len);
> +        nBufferLen +=3D copy_len;
> +        /* start DEBUG purpouse*/
> +//         rt_printk("Message Received: ");
> +//         for(nI =3D 0; nI <nBufferLen ; nI++)
> +//             rt_printk(" %02X",puchBuffer[nI]);
> +//         rt_printk("\n");
> +        /* end DEBUG purpouse*/
> +        /* copy the data to user */  =20
> +        rt_memcpy_tokerneliovec(msg->msg_iov, puchBuffer, nBufferLen);=

> +        /* new buffer length*/
> +        real_len =3D nBufferLen;
> +    }
> +    else
> +    {
> +        rt_printk("Packet: Socket type Not Supported\n");
> +    }
>      if ((msg_flags & MSG_PEEK) =3D=3D 0) {
>          rtdev_dereference(skb->rtdev);
>          kfree_rtskb(skb);
> @@ -383,12 +418,38 @@
> =20
>      rtskb->rtdev    =3D rtdev;
>      rtskb->priority =3D sock->priority;
> -
> -    if (rtdev->hard_header) {
> -        ret =3D rtdev->hard_header(rtskb, rtdev, ntohs(sll->sll_protoc=
ol),
> -                                 sll->sll_addr, rtdev->dev_addr, rtskb=
->len);
> -        if (ret < 0)
> -            goto err;
> +    // SOCK_DGRAM
> +    if(sockctx->device->socket_type =3D=3D SOCK_DGRAM)
> +    {
> +//         rt_printk("Packet:DEBUG: SOCKET DGRAM SELECTED\n");
> +       =20
> +        if (rtdev->hard_header) {
> +            ret =3D rtdev->hard_header(rtskb, rtdev, ntohs(sll->sll_pr=
otocol),
> +                                    sll->sll_addr, rtdev->dev_addr, rt=
skb->len);
> +            if (ret < 0)
> +                goto err;
> +        }
> +    }
> +    // SOCK_RAW
> +    else if (sockctx->device->socket_type =3D=3D SOCK_RAW)
> +    {
> +//         rt_printk("Packet:DEBUG: SOCKET RAW SELECTED\n");
> +        unsigned char puchBuffer[ETH_FRAME_LEN];
> +       =20
> +        if (rtdev->hard_header) {
> +            ret =3D rtdev->hard_header(rtskb, rtdev, (rtskb->data+ETH_=
ALEN+ETH_ALEN),
> +                                     sll->sll_addr, (rtskb->data+ETH_A=
LEN), (rtskb->len - ETH_HLEN));
> +            if (ret < 0)
> +                goto err;
> +        }
> +        memcpy(puchBuffer,rtskb->data,rtskb->len);
> +        memcpy(rtskb->data,(puchBuffer+ETH_HLEN),(rtskb->len - ETH_HLE=
N));
> +        rtskb->len =3D rtskb->len - ETH_HLEN;
> +//         rt_printk("Packet:DEBUG: SOCKET RAW SELECTED and exit\n");
> +    }
> +    else
> +    {
> +        rt_printk("Packet: Socket type Not Supported\n");

This is debug output, isn't it? If you get here, you did something wrong
during socket setup. That would also simplify the test above to

  if (...socket_type =3D=3D SOCK_DGRAM) { } else { }.

>      }
> =20
>      if ((rtdev->flags & IFF_UP) !=3D 0) {
> @@ -409,8 +470,6 @@
>      return ret;
>  }
> =20
> -
> -
>  static struct rtdm_device   packet_proto_dev =3D {
>      struct_version:     RTDM_DEVICE_STRUCT_VER,
> =20
> @@ -430,7 +489,7 @@
>          ioctl_nrt:      rt_packet_ioctl,
>          recvmsg_rt:     rt_packet_recvmsg,
>          sendmsg_rt:     rt_packet_sendmsg
> -    },
> +},
> =20
>      device_class:       RTDM_CLASS_NETWORK,
>      device_sub_class:   RTDM_SUBCLASS_RTNET,
> @@ -443,15 +502,55 @@
>  };
> =20
> =20
> +static struct rtdm_device   raw_packet_proto_dev =3D {
> +    struct_version:     RTDM_DEVICE_STRUCT_VER,
> +
> +    device_flags:       RTDM_PROTOCOL_DEVICE,
> +    context_size:       sizeof(struct rtsocket),
> +
> +    protocol_family:    PF_PACKET,
> +    socket_type:        SOCK_RAW,
> +
> +    socket_rt:          rt_packet_socket,
> +    socket_nrt:         rt_packet_socket,
> +
> +    ops: {
> +        close_rt:       rt_packet_close,
> +        close_nrt:      rt_packet_close,
> +        ioctl_rt:       rt_packet_ioctl,
> +        ioctl_nrt:      rt_packet_ioctl,
> +        recvmsg_rt:     rt_packet_recvmsg,
> +        sendmsg_rt:     rt_packet_sendmsg
> +    },
> +
> +    device_class:       RTDM_CLASS_NETWORK,
> +    device_sub_class:   RTDM_SUBCLASS_RTNET,
> +    driver_name:        "rtrawpacket",

Driver name is still "rtpacket" (same binary).

> +    driver_version:     RTNET_RTDM_VER,
> +    peripheral_name:    "Real-Time Packet Socket Interface",
> +    provider_name:      rtnet_rtdm_provider_name,
> +
> +    proc_name:          "PACKET_RAW"
> +};
> +
> +
>  int __init rt_packet_proto_init(void)
>  {
> -    return rtdm_dev_register(&packet_proto_dev);
> +    int nReturn =3D -1;
> +    nReturn =3D rtdm_dev_register(&packet_proto_dev);
> +    if(nReturn !=3D 0)
> +        return nReturn;
> +//     rt_printk("DGRAM return =3D %d\n",nReturn);
> +    nReturn =3D rtdm_dev_register(&raw_packet_proto_dev);
> +//     rt_printk("RAW return =3D %d\n",nReturn);
> +    return nReturn;

Mmh, and what happens to the packet_proto_dev registration if
raw_packet_proto_dev goes South?

>  }
> =20
> =20
>  void rt_packet_proto_release(void)
>  {
>      rtdm_dev_unregister(&packet_proto_dev, 1000);
> +    rtdm_dev_unregister(&raw_packet_proto_dev, 1000);
>  }
> =20
> =20

Thanks for your contribution!

Jan

[RTnet-developers] SOCKET RAW

[Jorge A. <j-a...@cr...>]

Jan,

In attach i send you a diff file regarding the af_packet.c file where the RAW 
SOCKET is implemented, similar to the DGRAM SOCKET.

I've already run some tests on it and everything looks OK.

Please run some more, or ask to (to some other interested person) and publish 
the results.


Now i'm starting with the ETH_P_ALL to the protocol definition.
Were, in the code, can i start to see the protocol filter implementation?
And wath is the best thing to do regarding ETH_P_ALL? Remove from QUEUE or 
copy the message from buffer and it still remains there? The second has the 
problem of the queue grows undefined. 
Any suggestions?


Thanks

-- 
Jorge Almeida
j-a...@cr...

DISCLAIMER: This message may contain confidential information or privileged material and is intended only for the individual(s) named. If you are not a named addressee and mistakenly received this message you should not copy or otherwise disseminate it: please delete this e-mail from your system and notify the sender immediately. E-mail transmissions are not guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete or contain viruses. Therefore, the sender does not accept liability for any errors or omissions in the contents of this message that arise as a result of e-mail transmissions. Please request a hard copy version if verification is required. Critical Software, SA.

Re: [RTnet-developers] Socket RAW

[Jan K. <jan...@we...>]

Jorge Almeida wrote:
> Jan,
>=20

[Even /me is sometimes busy - not with answering mails. ;)]

>=20
> I've looked in the af_packet.c file and i arrive to some conclusions fo=
r the RAW packet implementation, but i have some doubts regardind the rts=
kb struct.
>=20
> Especialy in:
>=20
> rtsckb.h - line 194
>     /* link layer */
>     union
>     {
>         struct ethhdr   *ethernet;
>         unsigned char   *raw;
>     } mac;
>=20
> i know that  "struct ethhdr   *ethernet; " keeps the source and destina=
tion mac address of the frame, but wath values "unsigned char   *raw;" ca=
n have?

It's a union, so you could also address the MAC as plain bytes this way.

>=20
>=20
> And another question,
> Does not existe any field in the rtskb struct that has all the Ethernet=
 frame inside (Destination Address, Source Address, Protocol, Data)? In t=
he driver everything is divided right?

mac.raw?

>=20
>=20
> One thing that i'm thinking is reconstruct a buffer with all this infor=
mation and return it to the user. This way the user has all the Ethernet =
frame in user space and can do anything he wants.

Please don't invent something new. Try to follow the existing
interfaces, maybe only skipping (yet) unneeded features.

Jan

[RTnet-developers] Socket RAW

[Jorge A. <j-a...@cr...>]

Jan,


I've looked in the af_packet.c file and i arrive to some conclusions for the RAW packet implementation, but i have some doubts regardind the rtskb struct.

Especialy in:

rtsckb.h - line 194
    /* link layer */
    union
    {
        struct ethhdr   *ethernet;
        unsigned char   *raw;
    } mac;

i know that  "struct ethhdr   *ethernet; " keeps the source and destination mac address of the frame, but wath values "unsigned char   *raw;" can have?


And another question,
Does not existe any field in the rtskb struct that has all the Ethernet frame inside (Destination Address, Source Address, Protocol, Data)? In the driver everything is divided right?


One thing that i'm thinking is reconstruct a buffer with all this information and return it to the user. This way the user has all the Ethernet frame in user space and can do anything he wants.


Please Comment my ideias.

Thanks
-- 
Jorge Almeida
j-a...@cr...
DISCLAIMER: This message may contain confidential information or privileged material and is intended only for the individual(s) named. If you are not a named addressee and mistakenly received this message you should not copy or otherwise disseminate it: please delete this e-mail from your system and notify the sender immediately. E-mail transmissions are not guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete or contain viruses. Therefore, the sender does not accept liability for any errors or omissions in the contents of this message that arise as a result of e-mail transmissions. Please request a hard copy version if verification is required. Critical Software, SA.