Hello,
some months ago (iirc) we had a discussion about the sip_msg structure and the fact that it has too many header hooks, some for pretty uncommon or not really used inside sip router code.
I spotted - subject - accept_language - content_disposition - accept_disposition - unsupported - organization - diversion (expired draft?)
The list can continue a bit ... I was thinking to replace them with a call of get_hdr_by_name|type() kind of function. The benefit will be lower structure.
On the other hand, I would add a new field to hold the pid of the process that received the message. There are some potential races by using only id to identify a message, especially in serial forking.
Opinions?
Daniel
On Feb 02, 2010 at 14:38, Daniel-Constantin Mierla miconda@gmail.com wrote:
Hello,
some months ago (iirc) we had a discussion about the sip_msg structure and the fact that it has too many header hooks, some for pretty uncommon or not really used inside sip router code.
I spotted
- subject
- accept_language
- content_disposition
- accept_disposition
- unsupported
- organization
- diversion (expired draft?)
The list can continue a bit ... I was thinking to replace them with a call of get_hdr_by_name|type() kind of function. The benefit will be lower structure.
On the other hand, I would add a new field to hold the pid of the process that received the message. There are some potential races by using only id to identify a message, especially in serial forking.
Opinions?
I fully agree. I've tried doing this before for ser, but hit very strong opposition.
Regarding the pid, I was thinking on extending the msg->id field and include the pid or process_no there. E.g.: struct msgid_str{ int msg_no; /* per process */ unsigned short reserved; /* future use (e.g. extending msg_no to 48 bits */ unsigned short proc_no; /* process number, 64k should be enough */ };
union msg_id{ struct msgid_str raw; unsigned long long id; }
typedef uninon msg_id msg_id_t;
Andrei
On 2/2/10 2:52 PM, Andrei Pelinescu-Onciul wrote:
On Feb 02, 2010 at 14:38, Daniel-Constantin Mierlamiconda@gmail.com wrote:
Hello,
some months ago (iirc) we had a discussion about the sip_msg structure and the fact that it has too many header hooks, some for pretty uncommon or not really used inside sip router code.
I spotted
- subject
- accept_language
- content_disposition
- accept_disposition
- unsupported
- organization
- diversion (expired draft?)
The list can continue a bit ... I was thinking to replace them with a call of get_hdr_by_name|type() kind of function. The benefit will be lower structure.
On the other hand, I would add a new field to hold the pid of the process that received the message. There are some potential races by using only id to identify a message, especially in serial forking.
Opinions?
I fully agree. I've tried doing this before for ser, but hit very strong opposition.
a common sip message has less than 20 headers, there are hooks for over 40 (not talking about IMS common cases, ok? :-) anyhow those are not hooked).
Regarding the pid, I was thinking on extending the msg->id field and include the pid or process_no there. E.g.: struct msgid_str{ int msg_no; /* per process */ unsigned short reserved; /* future use (e.g. extending msg_no to 48 bits */ unsigned short proc_no; /* process number, 64k should be enough */ };
union msg_id{ struct msgid_str raw; unsigned long long id; }
typedef uninon msg_id msg_id_t;
good solution indeed.
Daniel
On 2/2/10 8:52 PM, Juha Heinanen wrote:
Daniel-Constantin Mierla writes:
- diversion (expired draft?)
yes, but very widely implemented and used.
does not mean the support is removed. Just instead of using a special hook for the headers I listed, a function is called to locate it, which is iteration through headers list, compare by id or if id does not exist by name - therefore penalty would be barely noticeable. Average number of headers is probably 15 or less.
On the other hand, I have nothing against keeping some.
Cheers, Daniel