| 07:16.54 | *** join/#asterisk-dev CELYA_ (~Thunderbi@2a01:e0a:d:1111:b749:768d:ed77:7c9a) |
| 07:40.18 | *** join/#asterisk-dev pchero_work (~pchero@87.213.247.82) |
| 09:44.04 | *** join/#asterisk-dev jkroon (~jkroon@165.16.203.107) |
| 10:53.26 | *** join/#asterisk-dev jkroon_ (~jkroon@165.16.203.107) |
| 11:26.28 | *** join/#asterisk-dev hehol (~hehol@gatekeeper.loca.net) |
| 11:45.15 | jkroon_ | is anybody aware of a way to make pointers to pj_sockaddr and ast_sockaddr be incompatible? |
| 11:49.15 | file | I haven't tried or investigated such things, but I treat them as abstractions and don't take compatibility for granted |
| 11:49.40 | *** join/#asterisk-dev jkroon_ (~jkroon@165.16.203.107) |
| 11:51.17 | jkroon | file, they're not compatible, but if you accidentally pass a pointer of ast_sockaddr where a pointer to pj_sockaddr is required ... in my case at least I'm not even getting a warning. |
| 11:51.35 | file | I'd expect the compiler to complain unless it was voided |
| 11:51.52 | jkroon | so would i ... |
| 11:53.55 | jkroon | busy reworking the way ice host candidates are replaced ... need blacklist to be applied before adding into the limited buffer used by sock pj, which means so does alternate host candidates ... |
| 12:19.47 | jkroon | for main/acl.c, ast_apply_acl will anybody object to suppression of the rejection messages if the purpose is NULL? Currently all users of this in the codebase pass a non-NULL string. |
| 12:23.04 | file | why would you want to suppress messages? |
| 12:23.59 | jkroon | file, i'm using that function to filter ICE and STUN candidates. I've got >100 addresses on my host with only a handful being whitelisted. So for every single ICE-using call setup it logs >100 lines to my logs. |
| 12:24.46 | jkroon | also why i'm re-working some of the logic ... only the first 32 addresses on a host are considered for ICE ... |
| 12:25.05 | jkroon | instead of working through the whole list until we have our max of 32 candidates. |
| 12:25.19 | file | I'd rather not use the reason to dictate that behavior |
| 12:26.24 | file | or purpose rather |
| 12:27.18 | file | using purpose to indicate that is convenient, but that's not the purpose of purpose and the intent is not explicit |
| 12:31.38 | jkroon | i do actually aggree with that. the other option is adding an argument ... which requires a rather intrusive patch in comparison though. but I'm happy to rather do that if it's considered "the right thing to do". |
| 12:32.18 | file | breaking ABI wouldn't be acceptable for previous branches, but you can add additional |
| 12:33.15 | jkroon | there is probably only around 20 or so users ... so whilst much more intrusive it's also not a major issue. however, this is the only purpose for which the purpose argument is used ... so logprefix may be a better name currently, and if not specified, don't log. makes sense to me but I hear you. |
| 12:33.53 | jkroon | adding an argument breaks ABI does it not? |
| 12:34.00 | file | it does |
| 12:34.04 | file | I meant adding an additional function |
| 12:34.08 | file | or some other mechanism |
| 12:34.32 | file | should your change be WIP for now? |
| 12:34.56 | jkroon | enum ast_acl_sense ast_apply_acl <-- current, so add enum ast_acl_sense ast_apply_acl_no_log? |
| 12:35.00 | jkroon | yes, i'll change to WIP. |
| 12:35.31 | jkroon | but i don't think the first of the patches (one currently on gerrit) is going to change. |
| 12:36.02 | jkroon | acceptable to push a multi-commit change to gerrit? |
| 12:36.21 | file | adding a function like no_log is fine |
| 12:36.25 | file | and that depends on the change... |
| 12:36.35 | file | generally it's easiest to do 1 commit |
| 12:37.28 | jkroon | ok, so I've got (1) replace blacklist with acl, (2) move ice_host_candidates to load time instead of advertise time, and (3) move blacklisting to load time instead of advertise time. |
| 12:37.44 | jkroon | then intent was (4) silence pointless ACL failure logs for ICE and STUN. |
| 12:37.59 | file | you can do it across multiple commits, it just gets complicated to make changes and keep it up to date due to dependencies |
| 12:38.14 | file | unless they are kept independent then it's easy |
| 12:38.17 | jkroon | personally i think squashing all of that into a single change is over-complicating a single commit. |
| 12:39.04 | file | then you can do it separate |
| 12:39.10 | jkroon | ok, i've got them onto a single branch for now (and I'm fairly familiar with git magic to rebase) so let's see how it goes. If needed I'll either squash or split. |
| 12:44.45 | jkroon | if it's possible to resolve a forward declaration by way of moving a function rather than adding a prototype - is that preferred? |
| 12:46.36 | file | dependent on the code layout and usage |
| 12:47.22 | jkroon | is there guidelines or is it "use your best judgement?" |
| 12:47.56 | file | coding guidelines in general at https://wiki.asterisk.org/wiki/display/AST/Coding+Guidelines - I don't believe there is anything specific for this, so it'd be best judgment |
| 12:53.03 | jkroon | thanks for the advise file. appreciated. |
| 12:53.59 | jkroon | just going to put this through some basic sanity deployments, not quite in production (first patch is being used in production and is fine as long as the desired addresses are in the first 32 ... which generally tends to not be the case) |
| 13:04.12 | *** join/#asterisk-dev gtjoseph (~gtjoseph@unaffiliated/gtj) |
| 13:04.12 | *** mode/#asterisk-dev [+o gtjoseph] by ChanServ |
| 13:05.14 | seanbright | file: do you know what the media cache issue is? |
| 13:05.22 | seanbright | oh, nevermind. |
| 13:05.34 | seanbright | didn't see you changed the assignee |
| 13:05.45 | file | I don't but I like the change regardless, as it'll stop the messages that sometimes freak people out when they see them and don't know what it actually means |
| 13:05.50 | file | aye |
| 13:05.57 | file | I'm cleaning up the world |
| 13:06.08 | seanbright | i see that. |
| 13:06.19 | *** join/#asterisk-dev pc-m (~pcm@modemcable094.94-70-69.static.videotron.ca) |
| 13:09.44 | seanbright | open to suggestions on 'match' syntax to include ports |
| 13:10.33 | file | um, /<port>? >_> |
| 13:10.47 | file | oh, except yeah... |
| 13:10.50 | seanbright | so 1.2.3.0/24/5060 |
| 13:10.51 | seanbright | ? |
| 13:10.53 | file | strings are fun |
| 13:11.06 | file | that's the thing that immediately comes to mind, but I'm circling back, I'm going through my queue in order vaguely |
| 13:11.12 | seanbright | 1.2.3.0/24/[5060,6060] |
| 13:11.24 | seanbright | i'm still in my PJs, so no rush |
| 13:12.00 | gtjoseph | ew to both of those comments. |
| 13:12.25 | seanbright | would you prefer i be naked? |
| 13:12.29 | seanbright | pick your poison |
| 13:12.59 | gtjoseph | hmmm. i haven't seen you in person |
| 13:13.03 | seanbright | heh |
| 13:13.21 | file | gmail do what I say, mark as read! |
| 13:14.37 | jkroon | seanbright, :<port> perhaps? (more natural what people are generally used to, albeit does mess with v6 address formatting) |
| 13:14.58 | seanbright | and if there are multiple ports? |
| 13:15.06 | seanbright | i guess you could repeat the whole thing |
| 13:15.25 | seanbright | 1.2.3.0:5060/24, 1.2.3.0:6060/24 |
| 13:15.36 | seanbright | ignoring the complexity of IPv6 |
| 13:18.13 | file | that seems reasonable? |
| 13:18.28 | seanbright | ok, i will go down that path |
| 13:18.39 | file | ast_sockaddr_parse should be able to take care of the port |
| 13:19.02 | gtjoseph | looks better than the other alternatives |
| 13:19.31 | seanbright | i don't have the source in front of me at the moment - do IPv6 addresses currently need to be bracketed in that context? |
| 13:19.58 | file | it uses getaddrinfo underneath for it, so it'd be whatever it needs |
| 13:20.16 | seanbright | ok |
| 13:20.47 | seanbright | i would like some opinions on the patch attached to https://issues.asterisk.org/jira/browse/ASTERISK-28562 |
| 13:20.51 | file | probably not... |
| 13:21.46 | file | seanbright: seems fine to me at first glance? |
| 13:21.51 | seanbright | ok |
| 13:22.06 | seanbright | i'll get it ready for gerrit then |
| 13:22.28 | seanbright | i think we throw away iostreams for 18, but i will make a case for that at a later time |
| 13:22.35 | gtjoseph | speaking of gerrit. i'm going a quick upgrade. |
| 13:25.23 | file | seanbright: something new and better?!? |
| 13:26.05 | seanbright | unclear at this time |
| 13:26.35 | seanbright | it's a leaky abstraction |
| 13:27.33 | seanbright | it's odd that 99% of what we do is I/O and we don't do it consistently |
| 13:28.15 | file | except for the iostream thing from that individual it hasn't really been a focus explicitly of anyone, all the older stuff was a bit organic at the time |
| 13:28.22 | seanbright | yeah |
| 13:29.45 | seanbright | well, anything using iostreams and TLS is probably (at least a little) broken |
| 13:30.26 | file | wonders if that includes seanbright |
| 13:30.33 | seanbright | but anyway, i will come up with some kind of proposal |
| 13:30.39 | seanbright | libevent is where i am at currently |
| 13:31.12 | seanbright | but! i must get ready for work |
| 13:31.13 | seanbright | back in a bit |
| 13:36.22 | gtjoseph | gerrit and jenkins both upgraded successfully and back on line |
| 13:40.38 | *** join/#asterisk-dev sduthil (~sduthil@modemcable094.94-70-69.static.videotron.ca) |
| 13:47.44 | *** join/#asterisk-dev sduthil (~sduthil@modemcable094.94-70-69.static.videotron.ca) |
| 14:38.00 | seanbright | nice |
| 14:43.33 | jkroon | seanbright, 1.2.3.4/24:[5060,6000] ? |
| 14:44.26 | seanbright | eh |
| 14:44.37 | seanbright | so, ast_sockaddr_parse accepts a:b:c:...:d |
| 14:45.10 | seanbright | which means that a port spec might be unparseable if using a colon delimiter |
| 14:45.43 | jkroon | ok well, it was just a suggestion. v6 bites again :) |
| 14:46.02 | file | unless it required [] scoping |
| 14:46.17 | seanbright | that would be a behavior change? |
| 14:46.25 | file | only if a port is specified? |
| 14:46.31 | seanbright | good point |
| 14:46.57 | seanbright | ok, so we're saying: ip(:port)?(/mask)? |
| 14:47.03 | seanbright | that sound about right? |
| 14:47.53 | file | does to me |
| 14:48.09 | seanbright | okie |
| 14:48.30 | seanbright | i was thinking about the iostreams thing a bit more this morning after i mentioned it |
| 14:49.02 | seanbright | while i don't really like iostreams, i think we might be able to work around the weirdness if we change how we read |
| 14:49.19 | seanbright | in the websockets case, we have this today: |
| 14:49.32 | seanbright | while (wait_for_fd(x)) { read_one_message(x); } |
| 14:49.40 | seanbright | if instead we did: |
| 14:49.55 | seanbright | while (wait_for_fd(x)) { while (read_one_message(x)) { ... } } |
| 14:50.01 | seanbright | that would probably fix it without API changes |
| 14:50.57 | seanbright | we assume that when ast_wait_for_input() returns 1 that we have one message to read, when we might actually have more than 1. |
| 14:51.08 | seanbright | anyway, i will stop flooding the channel :) |
| 14:53.12 | file | hrm |
| 14:55.05 | seanbright | the way we read websocket messages is a bit weird too, but that's a different matter |
| 14:55.37 | seanbright | i think the ws_safe_read() made (more?) sense when we owned the fd (pre-iostreams) |
| 14:57.55 | seanbright | also, acls don't support ports, so this is going to suck big time |
| 14:58.45 | seanbright | i'll just keep a parallel vector |
| 14:59.05 | seanbright | (you can tell that i have been saving all of this since you were all on holiday) |
| 15:07.05 | file | offers seanbright a blueberry |
| 15:13.14 | *** join/#asterisk-dev pchero_work (~pchero@87.213.247.82) |
| 15:18.36 | *** join/#asterisk-dev bford (uid283514@gateway/web/irccloud.com/x-yuzqxscmllualfzh) |
| 15:18.36 | *** mode/#asterisk-dev [+o bford] by ChanServ |
| 16:16.14 | seanbright | i try to avoid anti-oxidants |
| 16:16.26 | seanbright | i prefer high toxin levels |
| 16:17.02 | file | oic |
| 18:02.01 | *** join/#asterisk-dev pc-m (~pcm@modemcable094.94-70-69.static.videotron.ca) |
| 18:08.17 | *** join/#asterisk-dev rmudgett (rmudgett@nat/digium/x-sslblvgjzigxgnrx) |
| 18:08.17 | *** mode/#asterisk-dev [+o rmudgett] by ChanServ |
| 18:49.01 | *** join/#asterisk-dev CELYA_ (~Thunderbi@2a01cb0583d31d00f4d7ddd1bc0987bc.ipv6.abo.wanadoo.fr) |
| 19:46.22 | jkroon | right, so i think i finally figured out why my secondary patches for ICE aren't working. So we're requesting PJSIP to give us a list of up to 32 addresses, and then we're filtering on those. In my case I've got a great many number more. Any chance to iterate the addresses in a callback style manner where we can then decide whether or not to add them, and return some indicator to indicate "stop iterating we can't handle any more"? |
| 19:48.37 | jkroon | is it an absolute requirement to utilize pjsip here, or is it simply that pjsip already did all of the heavy lifting and we'd prefer to not re-invent the wheel? |
| 19:51.54 | rmudgett | I think it was PJPROJECT did the heavy lifting. |
| 19:56.13 | jkroon | rmudgett, so if I re-implement that using getifaddrs(3)? Not sure how portable that is though ... the documentation seems to indicate "fairly" ... so since I only have Linux to test on, how can I test portability? |
| 19:57.23 | *** join/#asterisk-dev seanbright_ (~sean@asterisk/community-developer/seanbright) |
| 19:57.23 | *** mode/#asterisk-dev [+o seanbright_] by ChanServ |
| 19:57.31 | jkroon | my issue is probably fairly unique, I have >32 addresses on my host and the current code gets it wrong. |
| 19:58.07 | rmudgett | file could best answer that since he implemented the Asterisk ICE support |
| 19:58.26 | jkroon | my other option is to pass a much larger array to pj_enum_ip_interface - but I have no way of knowing how large that array will need to be. |
| 19:58.34 | rmudgett | That is a lot of interfaces |
| 19:59.27 | jkroon | bunch of v4 and v6 addresses, up to (currently) around 320 in total. rather complex setup I agree, and if I could simplify I would. but that's probably a bigger issue that patching asterisk. |
| 19:59.46 | jkroon | file, opinions? |
| 20:04.25 | jkroon | i'm also inclined to cache either the list of local addresses (return from getifaddrs) or the generated ICE candidate list, even if only for a second or two. |
| 20:05.46 | *** join/#asterisk-dev gtjoseph (~gtjoseph@unaffiliated/gtj) |
| 20:05.46 | *** mode/#asterisk-dev [+o gtjoseph] by ChanServ |
| 20:06.50 | *** join/#asterisk-dev pc-m (~pcm@modemcable094.94-70-69.static.videotron.ca) |
| 20:06.54 | file | if the end result is the same for host candidates then doing it some other way, as long as it's portable or dependent on platform, seems fine |
| 20:07.18 | file | I don't know in regards to passing a higher count to the pj function, never looked at it |
| 20:07.31 | jkroon | "dependent on platform" ? |
| 20:07.55 | jkroon | i just looked at the code, there should be no issue to passing it a really high (thousands) value for count. |
| 20:08.17 | file | if you were to write code it either has to work across platforms, or be ifdeffed such that things fallback or still work on other platforms |
| 20:08.34 | jkroon | it' only got variance for symbian os, otherwise it attempts two or three different mechanisms depending on what libraries/interfaces it's got available. |
| 20:08.57 | jkroon | ok, i'll see what I can do in the morning thanks file. |
| 20:10.26 | jkroon | file, i'm assuming I don't need to care too much (if at all) about portability to windows? |
| 20:10.37 | file | you don't |
| 20:10.49 | file | my comment was more in regards to you writing something yourself or using non-pj functions |
| 20:10.52 | file | not using the pj stuff |
| 20:11.29 | jkroon | yea, just wondering what platforms we care about, linux and bsd seems to be OK using getifaddrs, windows not. unsure what other platforms I need to care about. |
| 20:12.06 | jkroon | although, on BSD we'll only get a single address per interface, which is probably NOT what we want. So #ifdef looks like it's going to be the way to go. |
| 20:12.13 | file | Linux is first class, other platforms such as FreeBSD/OpenBSD are best effort |
| 20:12.29 | file | but they should not knowingly be broken or altered |
| 20:12.38 | file | to the deteriment of them |
| 20:12.45 | jkroon | it would be nice to not break something that's currently working. |
| 20:13.15 | jkroon | is there someone around using *BSD that would be able (and willing) to test final patch for me w.r.t. ICE? |
| 20:38.34 | *** join/#asterisk-dev CELYA_1 (~Thunderbi@lfbn-nan-1-68-217.w90-49.abo.wanadoo.fr) |
| 21:10.17 | seanbright_ | no one uses bsd |
| 21:10.19 | seanbright_ | i checked |
| 23:15.57 | *** join/#asterisk-dev pc-m (~pcm@cable-66-129-132-216.b2b2c.ca) |