pua: different behavior depending on db_mode #2414
Comments
|
@juha-h since you committed 6822ff4, can you please comment why it was necessary? Why on PUA_DB_ONLY there's no call to |
|
Victor Seva writes:
I don't remember. It was long time ago. Perhaps there has been some
mailing list discussion related to the commit.
Why on PUA_DB_ONLY there's no call to ``get_record_puadb``
6822ff4#diff-0c68f4211a54ae66988228c3e4b61852R521-R528
There is call to ``get_record_puadb``:
```
if (dbmode==PUA_DB_ONLY)
{
if (publ->etag) {
memset(&dbpres, 0, sizeof(dbpres));
dbpres.pres_uri = &pres_uri;
dbpres.watcher_uri = &watcher_uri;
dbpres.extra_headers = &extra_headers;
presentity = get_record_puadb(publ->id, publ->etag,
&dbpres, &res);
}
}
```
|
Well I mean if |
|
https://lists.kamailio.org/pipermail/sr-users/2016-January/091492.html
I still see this behavior and I think is related to this code since pua_dialoginfo never sets etag when calling |
|
Victor Seva writes:
@juha-h
> There is call to ``get_record_puadb``:
Well I mean if ``publ->etag`` is NULL, the previous behavior was to
call it always
I don't remember publish details, but if there is no etag, it is the
first publish and therefore it doesn't make sense to get the record from
db.
|
|
Victor Seva writes:
https://lists.kamailio.org/pipermail/sr-users/2016-January/091492.html
> pua - db_mode set to 0. This stops multiple states for a single dialog (early, trying, confirmed and terminated) from showing in the presentity table. I suspect this is a bug?
I still see this behavior and I think is related to this code since pua_dialoginfo never sets etag when calling ``send_publish()``
Then isn't it a bug in pua_dialoginfo?
If I remember correctly, first PUBLISH is sent without SIP-If-Match
header (containing etag), then etag is obtained from 200 OK's SIP-ETag
header and placed in SIP-If-Match header of following PUBLISH requests.
|
I don't think so. The purpose of E-Tag is to identify a state of the resource. If the resource changes a new E-Tag has to be generated. Generating Entity-tags |
|
Victor Seva writes:
I don't think so. The purpose of E-Tag is to identify a state of the
resource. If the resource changes a new E-Tag has to be
generated. [Generating
Entity-tags](https://tools.ietf.org/html/rfc5839#page-20)
That is about notifier behavior in subscribe/notify exchange and does
not deal with publish requests.
… In this case, pua_dialoginfo is changing the state so a new E-Tag has to be generated and the record in db in this case must be updated. Not insert a new one.
|
|
Ok. Then it should be this one. https://tools.ietf.org/html/rfc3903#section-8.2 The Even if So, with db_mode == 0: second PUBLISH But with db_mode == 2: second PUBLISH: |
linuxmaniac
added a commit
that referenced
this issue
Jul 30, 2020
* dialog PUBLISH was missing SIP-If-Mach * pua was inserting a new record for every dialog state fixes #2414
10 tasks
|
I no longer see more than one record in pua table and PUBLISH now have the proper |
|
other side effect is that only the last record of DIALOG_PUBLISH.* gets removed properly and the rest of records just are hanging there until max dialog duration time expires, by default 3600 secs |
|
I cannot reproduce the issue. In my tests after the first publish the rest
do have SIP-If-Match header, for example:
```
T 2020/07/30 20:57:06.251465 127.0.0.1:50064 -> 127.0.0.1:5080 [AP] #20
PUBLISH sip:foo@test.tutpro.com SIP/2.0.
Via: SIP/2.0/TCP 192.168.43.82;branch=z9hG4bK549a.f9d86324000000000000000000000000.0.
To: <sip:foo@test.tutpro.com>.
From: <sip:foo@test.tutpro.com>;tag=ded145cd44a1108aee8ba0e30b80344c-7d4055df.
CSeq: 10 PUBLISH.
Call-ID: 24b1a5bf25ad843a-12771@192.168.43.82.
Content-Length: 94.
User-Agent: OpenSIPg SIP Proxy (5.4.0-0b01 (x86_64/linux)).
Max-Forwards: 70.
Event: message-summary.
Expires: 7776001.
SIP-If-Match: a.1594729867.26073.6.12.
Content-Type: application/simple-message-summary.
P-Flags: 0.
.
Messages-Waiting: yes.
Message-Account: sip:foo@vm.test.tutpro.com.
Voice-Message: 1/0 (0/0).
```
That happens because etag param is included in the json request:
```
Jul 30 20:57:06 char /usr/bin/sip-proxy[12771]: INFO: Executing JSON request <{"jsonrpc":"2.0","method":"pua.publish","params":["sip:foo@test.tutpro.com","7776000","message-summary","application\/simple-message-summary",".","a.1594729867.26073.6.12","sip:127.0.0.1:5080;transport=tcp","P-Flags: 0\r\n","Messages-Waiting: yes\r\nMessage-Account: sip:foo@vm.test.tutpro.com\r\nVoice-Message: 1\/0 (0\/0)\r\n"],"id":1}> from <127.0.0.1> with host <127.0.0.1:6060>
```
Why can't pua_dialoginfo do the same?
|
|
Isn't it so that pua module send_publish function sends exactly such publish as specified in its parameters? One of the parameters is etag. So if first or subsequent publish requests contain SIP-If-Match header is up to what kind of parameters send_publish has got. |
|
Sorry @juha-h but I don't get your point. If a pua_* module execute send_publish() and etag is not set pua has to search anyway for that All of that doesn't happen with da7f7ef Every pua_* module that uses pua.send_publish() except pua_rpc doesn't set etag parameter.
So I don't get what you want to achieve. This was a bug that users reported and I had suffered. |
|
Victor Seva writes:
All of that doesn't happen with da7f7ef
Every **pua_*** module that uses pua.send_publish() except **pua_rpc** doesn't set etag parameter.
* **pua_bla**
* **pua_dialoginfo**
* **pua_usrloc**
* **pua_xmpp**
So I don't get what you want to achieve. This was a bug that users
reported and I had suffered.
Why don't these other modules keep track of the etag received from 200
ok to the initial publish and use it in subsequent ones?
Are you sure that the change you made does not have any negative
consequences to pua_rpc user that saves the etag from the initial
publish and uses it in subsequent ones?
May be there was some real reason when I made the commit many years
ago.
|
linuxmaniac
added a commit
that referenced
this issue
Jul 31, 2020
* only relevant when db_mode is PUA_DB_ONLY * call_id/to_tag/from_tag values can be "", for instance with DIALOG_PUBLISH.* records. Then **ALL** records get version field update * update_vesion_puadb() is called from send_publish() and pres->id value is valid after a call to get_record_puadb() related to #2414
10 tasks
I cannot be sure 100% but from what I understand if you set etag... it has the same behavior than previously. Change is merged already do you detect any regression? |
|
Victor Seva writes:
I cannot be sure 100% but from what I understand if you set etag... it has the same behavior than previously.
If is the first PUBLISH, there's no record, so no changes in logic.
Change is merged already do you detect any regression?
I can test when the merge is backported to 5.4, since I don't at present
have a live master system.
|
linuxmaniac
added a commit
that referenced
this issue
Aug 1, 2020
* only relevant when db_mode is PUA_DB_ONLY * call_id/to_tag/from_tag values can be "", for instance with DIALOG_PUBLISH.* records. Then **ALL** records get version field update * update_vesion_puadb() is called from send_publish() and pres->id value is valid after a call to get_record_puadb() related to #2414
linuxmaniac
added a commit
that referenced
this issue
Aug 1, 2020
* only relevant when db_mode is PUA_DB_ONLY * call_id/to_tag/from_tag values can be "", for instance with DIALOG_PUBLISH.* records. Then **ALL** records get version field update * update_vesion_puadb() is called from send_publish() and pres->id value is valid after a call to get_record_puadb() related to #2414 (cherry picked from commit e4aed5c)
|
Juha Heinanen writes:
Victor Seva writes:
> I cannot be sure 100% but from what I understand if you set etag... it has the same behavior than previously.
> If is the first PUBLISH, there's no record, so no changes in logic.
>
> Change is merged already do you detect any regression?
I can test when the merge is backported to 5.4, since I don't at present
have a live master system.
I installed latest master and looks like pua_rpc pua_publish with
explicit etag args works OK.
…-- Juha
|
linuxmaniac
added a commit
that referenced
this issue
Aug 3, 2020
* only relevant when db_mode is PUA_DB_ONLY * call_id/to_tag/from_tag values can be "", for instance with DIALOG_PUBLISH.* records. Then **ALL** records get version field update * update_vesion_puadb() is called from send_publish() and pres->id value is valid after a call to get_record_puadb() related to #2414 (cherry picked from commit 733f5f2)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
If db_mode == 2 (PUA_DB_ONLY)
SIP-If-Matchheader doesn't exists on dialog eventsame escenario but with db_mode == 0
Log Messages
with db_mode == 0
with db_mode == 2
Possible Solutions
I think the problem here is that on db_mode == 2 pua never search for the pua record if no etag is set
change introduced at 6822ff4
Additional Information
kamailio -vThis is Sipwise's kamailio version based on 5.3.5
The text was updated successfully, but these errors were encountered: