[Click] ANNOUNCE: Click-1.7.0rc1

[Beyers Cronje]

Hi Eddie,

I understand adding more data to the Packet header is not desirable and I'm
happy not to go that route.

I think I have found an alternative way to accomplish what I want to do
without changing Packet, so please disregard my request :)

Beyers

On Mon, Mar 9, 2009 at 4:30 PM, Eddie Kohler <kohler at cs.ucla.edu> wrote:

> Hi Beyers,
>
> Can you not use an annotation to store this type of data?  I'd rather not
> continue adding data to a Packet header.
>
> Eddie
>
>
>
> Beyers Cronje wrote:
>
>> Hi Eddie,
>>
>> I use the userlevel Packet destructor callback and needed a way to store
>> user data to be returned via the destructor callback. Any chance something
>> like this might be consired for inclusion in 1.7 ?
>>
>> Note the patch below was generated on an older version of Click, so it
>> will probable not apply directly anymore. I'm just curious IF you would
>> consider such a change? If so, then I will gladly generate a new patch on
>> current GIT version.
>>
>> Cheers
>>
>> Beyers
>>
>> diff -Naur click/elements/userlevel/fromfile.cc
>> newclick/elements/userlevel/fromfile.cc
>> --- click/elements/userlevel/fromfile.cc    2007-09-18 22:37:05.000000000
>> +0200
>> +++ newclick/elements/userlevel/fromfile.cc    2007-10-16
>> 21:21:43.000000000 +0200
>> @@ -120,7 +120,7 @@
>>   #ifdef ALLOW_MMAP
>>  static void
>> -munmap_destructor(unsigned char *data, size_t amount)
>> +munmap_destructor(unsigned char *data, size_t amount, void *)
>>  {
>>     if (munmap((caddr_t)data, amount) < 0)
>>     click_chatter("FromFile: munmap: %s", strerror(errno));
>> @@ -158,7 +158,7 @@
>>     if (mmap_data == MAP_FAILED)
>>     return error(errh, "mmap: %s", strerror(errno));
>>  -    _data_packet = Packet::make((unsigned char *)mmap_data, _len,
>> munmap_destructor);
>> +    _data_packet = Packet::make((unsigned char *)mmap_data, _len,
>> munmap_destructor, 0);
>>     _buffer = _data_packet->data();
>>     _file_offset = _mmap_off;
>>     _mmap_off += _len;
>> diff -Naur click/include/click/packet.hh newclick/include/click/packet.hh
>> --- click/include/click/packet.hh    2007-09-07 22:26:45.000000000 +0200
>> +++ newclick/include/click/packet.hh    2007-10-16 21:17:52.000000000
>> +0200
>> @@ -21,12 +21,12 @@
>>  # include <click/simclick.h>
>>  #endif
>>  -
>>  CLICK_DECLS
>>   class IP6Address;
>>  class WritablePacket;
>>  +
>>  class Packet { public:
>>     // PACKET CREATION
>> @@ -51,7 +51,7 @@
>>   const struct mbuf *m() const        { return (const struct mbuf *)_m; }
>>   struct mbuf *steal_m();
>>  #else            /* User-space */
>> -  static WritablePacket *make(unsigned char *, uint32_t, void
>> (*destructor)(unsigned char *, size_t));
>> +  static WritablePacket *make(unsigned char *, uint32_t, void
>> (*destructor)(unsigned char *, size_t, void *), void *);
>>  #endif
>>       inline void kill();
>> @@ -81,6 +81,7 @@
>>  #if CLICK_USERLEVEL
>>     inline void shrink_data(const unsigned char *, uint32_t length);
>>     inline void change_headroom_and_length(uint32_t headroom, uint32_t
>> length);
>> +    inline void *client_data() { return _client_data; }
>>  #endif
>>       // HEADER ANNOTATIONS
>> @@ -255,7 +256,8 @@
>>     unsigned char *_tail; /* one beyond end of packet */
>>     unsigned char *_end;  /* one beyond end of allocated buffer */
>>  # if CLICK_USERLEVEL
>> -    void (*_destructor)(unsigned char *, size_t);
>> +    void (*_destructor)(unsigned char *, size_t, void *);
>> +    void *_client_data;
>>  # endif
>>     unsigned char _cb[48];
>>     unsigned char *_mac;
>> @@ -325,6 +327,21 @@
>>   };
>>  +inline void
>> +Packet::kill()
>> +{
>> +#if CLICK_LINUXMODULE
>> +    struct sk_buff *b = skb();
>> +    b->next = b->prev = 0;
>> +# if LINUX_VERSION_CODE <= KERNEL_VERSION(2, 6, 15)
>> +    b->list = 0;
>> +# endif
>> +    skbmgr_recycle_skbs(b);
>> +#else
>> +    if (_use_count.dec_and_test())
>> +        delete this;
>> +#endif
>> +}
>>    inline const unsigned char *
>> @@ -656,23 +673,6 @@
>>  #endif
>>   -inline void
>> -Packet::kill()
>> -{
>> -#if CLICK_LINUXMODULE
>> -    struct sk_buff *b = skb();
>> -    b->next = b->prev = 0;
>> -# if LINUX_VERSION_CODE <= KERNEL_VERSION(2, 6, 15)
>> -    b->list = 0;
>> -# endif
>> -    skbmgr_recycle_skbs(b);
>> -#else
>> -    if (_use_count.dec_and_test())
>> -    delete this;
>> -#endif
>> -}
>> -
>> -
>>  #if CLICK_BSDMODULE        /* BSD kernel module */
>>  inline void
>>  Packet::assimilate_mbuf(Packet *p)
>> diff -Naur click/lib/packet.cc newclick/lib/packet.cc
>> --- click/lib/packet.cc    2007-09-07 19:53:46.000000000 +0200
>> +++ newclick/lib/packet.cc    2007-10-16 21:17:02.000000000 +0200
>> @@ -25,6 +25,7 @@
>>  #endif
>>  CLICK_DECLS
>>  +
>>  #ifdef CLICK_LINUXMODULE    /* Linux kernel module */
>>   Packet::Packet()
>> @@ -66,6 +67,7 @@
>>   _head = _data = _tail = _end = 0;
>>  #if CLICK_USERLEVEL
>>   _destructor = 0;
>> +  _client_data = 0;
>>  #elif CLICK_BSDMODULE
>>   _m = 0;
>>  #endif
>> @@ -78,7 +80,7 @@
>>     _data_packet->kill();
>>  #if CLICK_USERLEVEL
>>   else if (_head && _destructor)
>> -    _destructor(_head, _end - _head);
>> +    _destructor(_head, _end - _head, _client_data);
>>   else
>>     delete[] _head;
>>  #elif CLICK_BSDMODULE
>> @@ -97,13 +99,14 @@
>>  #ifdef CLICK_USERLEVEL
>>   WritablePacket *
>> -Packet::make(unsigned char *data, uint32_t len, void (*destruct)(unsigned
>> char *, size_t))
>> +Packet::make(unsigned char *data, uint32_t len, void (*destruct)(unsigned
>> char *, size_t, void *), void *cdata)
>>  {
>>   WritablePacket *p = new WritablePacket;
>>   if (p) {
>>     p->_head = p->_data = data;
>>     p->_tail = p->_end = data + len;
>>     p->_destructor = destruct;
>> +    p->_client_data = cdata;
>>   }
>>   return p;
>>  }
>> @@ -319,7 +322,7 @@
>>     _data_packet->kill();
>>  # if CLICK_USERLEVEL
>>   else if (_destructor)
>> -    _destructor(old_head, old_end - old_head);
>> +    _destructor(old_head, old_end - old_head, _client_data);
>>   else
>>     delete[] old_head;
>>   _destructor = 0;
>>
>>
>>