You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Paul Querna <ch...@force-elite.com> on 2007/11/12 21:03:14 UTC
[PATCH] UDP Bucket Support
Adds: apr_bucket_socket_get_peer() and has the bucket read call recvfrom().
Based on an earlier patch from Issac Goldstand.
Re: [PATCH] UDP Bucket Support
Posted by Issac Goldstand <ma...@beamartyr.net>.
Davi Arnaut wrote:
> Henry Jen wrote:
>
>> On Nov 12, 2007 6:41 PM, Davi Arnaut <da...@apache.org> wrote:
>>
>>> Paul Querna wrote:
>>>
>>>> Adds: apr_bucket_socket_get_peer() and has the bucket read call recvfrom().
>>>>
>>>> Based on an earlier patch from Issac Goldstand.
>>>>
>>>>
>>> [..]
>>>
>>>
>>>> - rv = apr_socket_recv(p, buf, len);
>>>> + apr_socket_type_get(p, &sd_type);
>>>> + if (sd_type == SOCK_STREAM) {
>>>> + rv = apr_socket_recv(p, buf, len);
>>>> + } else {
>>>> + /* Is socket connected? */
>>>> + if (apr_socket_addr_get(&baton->peer, APR_REMOTE, p) != APR_SUCCESS) {
>>>> + rv = apr_socket_recv(p, buf, len);
>>>> + } else {
>>>> + if (!baton->peer) {
>>>> + baton->peer = apr_bucket_alloc(sizeof(*baton->peer), a->list);
>>>> + }
>>>> + /* Caller is responsible for detecting peer on his own if needed */
>>>> + rv = apr_socket_recvfrom(baton->peer, p, 0, buf, len);
>>>> + }
>>>> + }
>>>>
>>> Why not cache the the peer and avoid a call to apr_socket_addr_get on
>>> every read?
>>>
>>>
>> UDP traffic can come from different peers.
>> I wonder if we can simply call apr_socket_recvfrom for all non-stream protocols?
>>
>
> I think you missed my point. Since the patch exports apr_bucket_socket
> why not let the user force a specific baton->peer (apr_sockaddr_t) ?
>
> Something like:
>
> } else {
> if (baton->peer || apr_socket_addr_get() == APR_SUCCESS)
> if (!baton->peer)
> baton->peer = apr_bucket_alloc();
> rv = apr_socket_recvfrom(baton->peer, p, 0, buf, len);
> else
> rv = apr_socket_recv(p, buf, len);
> }
>
I don't think that will do what you intend. Doing recvfrom won't
connect the socket in any way, so calls to apr_socket_recv won't work on
it later on. The same single socket needs to receive from *every* UDP peer.
Issac
Re: [PATCH] UDP Bucket Support
Posted by Davi Arnaut <da...@apache.org>.
Henry Jen wrote:
> On Nov 12, 2007 6:41 PM, Davi Arnaut <da...@apache.org> wrote:
>> Paul Querna wrote:
>>> Adds: apr_bucket_socket_get_peer() and has the bucket read call recvfrom().
>>>
>>> Based on an earlier patch from Issac Goldstand.
>>>
>> [..]
>>
>>> - rv = apr_socket_recv(p, buf, len);
>>> + apr_socket_type_get(p, &sd_type);
>>> + if (sd_type == SOCK_STREAM) {
>>> + rv = apr_socket_recv(p, buf, len);
>>> + } else {
>>> + /* Is socket connected? */
>>> + if (apr_socket_addr_get(&baton->peer, APR_REMOTE, p) != APR_SUCCESS) {
>>> + rv = apr_socket_recv(p, buf, len);
>>> + } else {
>>> + if (!baton->peer) {
>>> + baton->peer = apr_bucket_alloc(sizeof(*baton->peer), a->list);
>>> + }
>>> + /* Caller is responsible for detecting peer on his own if needed */
>>> + rv = apr_socket_recvfrom(baton->peer, p, 0, buf, len);
>>> + }
>>> + }
>> Why not cache the the peer and avoid a call to apr_socket_addr_get on
>> every read?
>>
>
> UDP traffic can come from different peers.
> I wonder if we can simply call apr_socket_recvfrom for all non-stream protocols?
I think you missed my point. Since the patch exports apr_bucket_socket
why not let the user force a specific baton->peer (apr_sockaddr_t) ?
Something like:
} else {
if (baton->peer || apr_socket_addr_get() == APR_SUCCESS)
if (!baton->peer)
baton->peer = apr_bucket_alloc();
rv = apr_socket_recvfrom(baton->peer, p, 0, buf, len);
else
rv = apr_socket_recv(p, buf, len);
}
Regards,
--
Davi Arnaut
Re: [PATCH] UDP Bucket Support
Posted by Henry Jen <he...@ztune.net>.
On Nov 12, 2007 6:41 PM, Davi Arnaut <da...@apache.org> wrote:
> Paul Querna wrote:
> > Adds: apr_bucket_socket_get_peer() and has the bucket read call recvfrom().
> >
> > Based on an earlier patch from Issac Goldstand.
> >
>
> [..]
>
> > - rv = apr_socket_recv(p, buf, len);
> > + apr_socket_type_get(p, &sd_type);
> > + if (sd_type == SOCK_STREAM) {
> > + rv = apr_socket_recv(p, buf, len);
> > + } else {
> > + /* Is socket connected? */
> > + if (apr_socket_addr_get(&baton->peer, APR_REMOTE, p) != APR_SUCCESS) {
> > + rv = apr_socket_recv(p, buf, len);
> > + } else {
> > + if (!baton->peer) {
> > + baton->peer = apr_bucket_alloc(sizeof(*baton->peer), a->list);
> > + }
> > + /* Caller is responsible for detecting peer on his own if needed */
> > + rv = apr_socket_recvfrom(baton->peer, p, 0, buf, len);
> > + }
> > + }
>
> Why not cache the the peer and avoid a call to apr_socket_addr_get on
> every read?
>
UDP traffic can come from different peers.
I wonder if we can simply call apr_socket_recvfrom for all non-stream protocols?
Cheers,
Henry
Re: [PATCH] UDP Bucket Support
Posted by Davi Arnaut <da...@apache.org>.
Paul Querna wrote:
> Adds: apr_bucket_socket_get_peer() and has the bucket read call recvfrom().
>
> Based on an earlier patch from Issac Goldstand.
>
[..]
> - rv = apr_socket_recv(p, buf, len);
> + apr_socket_type_get(p, &sd_type);
> + if (sd_type == SOCK_STREAM) {
> + rv = apr_socket_recv(p, buf, len);
> + } else {
> + /* Is socket connected? */
> + if (apr_socket_addr_get(&baton->peer, APR_REMOTE, p) != APR_SUCCESS) {
> + rv = apr_socket_recv(p, buf, len);
> + } else {
> + if (!baton->peer) {
> + baton->peer = apr_bucket_alloc(sizeof(*baton->peer), a->list);
> + }
> + /* Caller is responsible for detecting peer on his own if needed */
> + rv = apr_socket_recvfrom(baton->peer, p, 0, buf, len);
> + }
> + }
Why not cache the the peer and avoid a call to apr_socket_addr_get on
every read?
Regards,
Davi Arnaut