You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by Brian Neradt <br...@gmail.com> on 2020/08/28 14:40:59 UTC

TS API Review: New HTTP/2 stream id and priority getters

Hi dev@trafficserver.apache.org,

It would be helpful to add HTTP/2 stream and priority support to Traffic
Dump. In order for the plugin to access this information, I propose adding
the following two functions to the TS API:

tsapi TSReturnCode TSHttpTxnClientHttp2StreamIdGet(TSHttpTxn txnp, uint32_t
*stream_id);

tsapi TSReturnCode TSHttpTxnClientHttp2PriorityGet(TSHttpTxn txnp, int
*stream_dependency, int *weight);

I've created a draft PR with this implemented along with Traffic Dump's use
of it:
https://github.com/apache/trafficserver/pull/7149

Please let me know if there are any concerns or suggestions for improvement
concerning this.

Thanks!
Brian

-- 
"Come to Me, all who are weary and heavy-laden, and I will
give you rest. Take My yoke upon you and learn from Me, for
I am gentle and humble in heart, and you will find rest for
your souls. For My yoke is easy and My burden is light."

    ~ Matthew 11:28-30

Re: TS API Review: New HTTP/2 stream id and priority getters

Posted by Leif Hedstrom <zw...@apache.org>.

> On Aug 30, 2020, at 19:48, Masakazu Kitajo <ma...@apache.org> wrote:
> 
> I think we should avoid adding APIs just for HTTP/2 unless it's really
> HTTP/2 specific. Are we going to add TSHttpTxnClientHttp3StreamIdGet and
> TSHttpTxnClientHttp3PriorityGet as well?
> 
> I'd omit the "2" in the API names, and rephrase the API doc like:
> This API returns an error if the provided transaction does not have a
> stream id.
> 
> Also, a stream id on HTTP/3 (QUIC) is 62 bit integer.


+1. Lets make a continued effort to have as few APIs as possible. If needed, passing along a protocol identifier would be preferable, such that one API for all “streams” capable protocols can use the same API.


— Leif
> 
> Masakazu
> 
> 
>> On Sat, Aug 29, 2020 at 5:24 AM SUSAN HINRICHS <sh...@ieee.org> wrote:
>> 
>> +1
>> 
>> On Fri, Aug 28, 2020 at 3:23 PM Sudheer Vinukonda
>> <su...@yahoo.com.invalid> wrote:
>> 
>>> +1.
>>> Sounds reasonable to me.
>>> 
>>> 
>>>    On Friday, August 28, 2020, 07:41:18 AM PDT, Brian Neradt <
>>> brian.neradt@gmail.com> wrote:
>>> 
>>> Hi dev@trafficserver.apache.org,
>>> 
>>> It would be helpful to add HTTP/2 stream and priority support to Traffic
>>> Dump. In order for the plugin to access this information, I propose
>> adding
>>> the following two functions to the TS API:
>>> 
>>> tsapi TSReturnCode TSHttpTxnClientHttp2StreamIdGet(TSHttpTxn txnp,
>> uint32_t
>>> *stream_id);
>>> 
>>> tsapi TSReturnCode TSHttpTxnClientHttp2PriorityGet(TSHttpTxn txnp, int
>>> *stream_dependency, int *weight);
>>> 
>>> I've created a draft PR with this implemented along with Traffic Dump's
>> use
>>> of it:
>>> https://github.com/apache/trafficserver/pull/7149
>>> 
>>> Please let me know if there are any concerns or suggestions for
>> improvement
>>> concerning this.
>>> 
>>> Thanks!
>>> Brian
>>> 
>>> --
>>> "Come to Me, all who are weary and heavy-laden, and I will
>>> give you rest. Take My yoke upon you and learn from Me, for
>>> I am gentle and humble in heart, and you will find rest for
>>> your souls. For My yoke is easy and My burden is light."
>>> 
>>>    ~ Matthew 11:28-30
>>> 
>> 


Re: [E] Re: TS API Review: New HTTP/2 stream id and priority getters

Posted by Sudheer Vinukonda <su...@yahoo.com.INVALID>.
 +1

    On Tuesday, September 1, 2020, 03:49:43 PM PDT, Masaori Koshiba <ma...@apache.org> wrote:  
 
 > tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp,
TSHttpStreamId *stream_id);
> tsapi TSReturnCode TSHttpTxnClientStreamPriorityGet(TSHttpTxn txnp,
TSHttpPriority *priority);

+1

On Wed, Sep 2, 2020 at 7:35 AM Brian Neradt <br...@gmail.com> wrote:

> I updated the protocol draft with this input (notice I kept the updated API
> implementation as the second commit in the PR for the sake of comparison):
> https://github.com/apache/trafficserver/pull/7149
>
> The API now looks like the following:
>
> tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp,
> TSHttpStreamId *stream_id);
> tsapi TSReturnCode TSHttpTxnClientStreamPriorityGet(TSHttpTxn txnp,
> TSHttpPriority *priority);
>
> TSHttpStreamId and TSHttpPriority are now abstract structures that can
> reference TSHttp2StreamId and TSHttp2Priority objects. In the future we can
> expand this to include version 3 values of these. Notice that the above are
> generic to the specific HTTP protocol type used by the transaction.
>
> I'll hold off on updating the docs in the PR until I get confirmation that
> this looks OK to the community.
>
> Thanks,
> Brian
>
> On Tue, Sep 1, 2020 at 4:20 PM Brian Neradt <br...@gmail.com>
> wrote:
>
> > This sounds good to me. This essentially puts the type parameter in the
> > structure itself rather than as a separate parameter to the functions.
> >
> > On Tue, Sep 1, 2020 at 4:11 PM Alan Carroll
> > <so...@verizonmedia.com.invalid> wrote:
> >
> >> Sorry for chiming in late -
> >>
> >> Note this is extremely similar to IP addresses and I recommend we use
> the
> >> same solution. That is, there is a class HttpPriority which has just a
> >> type/style/family value and possibly a length. This is an abstract class
> >> like sockaddr (which no one actually instantiates). The family indicates
> >> the concrete type, which is something like Http2Priority or
> Http3Priority,
> >> etc., just like sockaddr_in and sockaddr_in6. When you call the priority
> >> getter, it returns a HttpPriority* which is a pointer to a concrete
> >> instance, the type of which is determined by the family. Again, just
> like
> >> getting an IP address from a socket.
> >>
> >
> >
> > --
> > "Come to Me, all who are weary and heavy-laden, and I will
> > give you rest. Take My yoke upon you and learn from Me, for
> > I am gentle and humble in heart, and you will find rest for
> > your souls. For My yoke is easy and My burden is light."
> >
> >    ~ Matthew 11:28-30
> >
>
>
> --
> "Come to Me, all who are weary and heavy-laden, and I will
> give you rest. Take My yoke upon you and learn from Me, for
> I am gentle and humble in heart, and you will find rest for
> your souls. For My yoke is easy and My burden is light."
>
>    ~ Matthew 11:28-30
>
  

Re: [E] Re: TS API Review: New HTTP/2 stream id and priority getters

Posted by Masaori Koshiba <ma...@apache.org>.
> tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp,
TSHttpStreamId *stream_id);
> tsapi TSReturnCode TSHttpTxnClientStreamPriorityGet(TSHttpTxn txnp,
TSHttpPriority *priority);

+1

On Wed, Sep 2, 2020 at 7:35 AM Brian Neradt <br...@gmail.com> wrote:

> I updated the protocol draft with this input (notice I kept the updated API
> implementation as the second commit in the PR for the sake of comparison):
> https://github.com/apache/trafficserver/pull/7149
>
> The API now looks like the following:
>
> tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp,
> TSHttpStreamId *stream_id);
> tsapi TSReturnCode TSHttpTxnClientStreamPriorityGet(TSHttpTxn txnp,
> TSHttpPriority *priority);
>
> TSHttpStreamId and TSHttpPriority are now abstract structures that can
> reference TSHttp2StreamId and TSHttp2Priority objects. In the future we can
> expand this to include version 3 values of these. Notice that the above are
> generic to the specific HTTP protocol type used by the transaction.
>
> I'll hold off on updating the docs in the PR until I get confirmation that
> this looks OK to the community.
>
> Thanks,
> Brian
>
> On Tue, Sep 1, 2020 at 4:20 PM Brian Neradt <br...@gmail.com>
> wrote:
>
> > This sounds good to me. This essentially puts the type parameter in the
> > structure itself rather than as a separate parameter to the functions.
> >
> > On Tue, Sep 1, 2020 at 4:11 PM Alan Carroll
> > <so...@verizonmedia.com.invalid> wrote:
> >
> >> Sorry for chiming in late -
> >>
> >> Note this is extremely similar to IP addresses and I recommend we use
> the
> >> same solution. That is, there is a class HttpPriority which has just a
> >> type/style/family value and possibly a length. This is an abstract class
> >> like sockaddr (which no one actually instantiates). The family indicates
> >> the concrete type, which is something like Http2Priority or
> Http3Priority,
> >> etc., just like sockaddr_in and sockaddr_in6. When you call the priority
> >> getter, it returns a HttpPriority* which is a pointer to a concrete
> >> instance, the type of which is determined by the family. Again, just
> like
> >> getting an IP address from a socket.
> >>
> >
> >
> > --
> > "Come to Me, all who are weary and heavy-laden, and I will
> > give you rest. Take My yoke upon you and learn from Me, for
> > I am gentle and humble in heart, and you will find rest for
> > your souls. For My yoke is easy and My burden is light."
> >
> >     ~ Matthew 11:28-30
> >
>
>
> --
> "Come to Me, all who are weary and heavy-laden, and I will
> give you rest. Take My yoke upon you and learn from Me, for
> I am gentle and humble in heart, and you will find rest for
> your souls. For My yoke is easy and My burden is light."
>
>     ~ Matthew 11:28-30
>

Re: [E] Re: TS API Review: New HTTP/2 stream id and priority getters

Posted by Alan Carroll <so...@verizonmedia.com.INVALID>.
Some notes:

maskit - we can just add another enum value. You're right that it should
probably not be "ProtocolType" because it may be unrelated. "PriorityType"
would probably be better as a name.

There shouldn't be a "0_9" - I thought we had agreed to not support that at
all anymore.

There should be an INVALID of some sort, equivalent to AF_UNSPEC.

I agree with maskit that the indirection isn't worth it for the ID.

On Tue, Sep 1, 2020 at 8:08 PM Masakazu Kitajo <ma...@apache.org> wrote:

> Actually, I don't understand TSHttpProtocolType. Shouldn't it be
> StructureType? Let's say we support a new priority scheme that is available
> on both H2 and H3. What would be the TSHttpProtocolType for it?
>
> Masakazu
>
> On Wed, Sep 2, 2020 at 9:37 AM Masakazu Kitajo <ma...@apache.org> wrote:
>
> > +1 I'm ok with the new function signatures that use abstract structures.
> >
> > However, I'm not sure if we really need that flexibility for Stream ID, I
> > can't say 64-bit is big enough, but I think extensibility is not
> > everything. Usability matters. Should we prepare for 128-bit stream ids
> > now? Would we take the same approach for TSUrlPortGet to prepare for
> 32-bit
> > port numbers?
> >
> > Masakazu
> >
> > On Wed, Sep 2, 2020 at 7:35 AM Brian Neradt <br...@gmail.com>
> > wrote:
> >
> >> I updated the protocol draft with this input (notice I kept the updated
> >> API
> >> implementation as the second commit in the PR for the sake of
> comparison):
> >>
> https://urldefense.com/v3/__https://github.com/apache/trafficserver/pull/7149__;!!Op6eflyXZCqGR5I!R1jXXdxiTw57gMmYf4tr78KcBgZfbbrmlMwrL-m5hwDyQg7Ue6tcXgslvNZ9bxq2Wr5rdqwl9w$
> >>
> >> The API now looks like the following:
> >>
> >> tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp,
> >> TSHttpStreamId *stream_id);
> >> tsapi TSReturnCode TSHttpTxnClientStreamPriorityGet(TSHttpTxn txnp,
> >> TSHttpPriority *priority);
> >>
> >> TSHttpStreamId and TSHttpPriority are now abstract structures that can
> >> reference TSHttp2StreamId and TSHttp2Priority objects. In the future we
> >> can
> >> expand this to include version 3 values of these. Notice that the above
> >> are
> >> generic to the specific HTTP protocol type used by the transaction.
> >>
> >> I'll hold off on updating the docs in the PR until I get confirmation
> that
> >> this looks OK to the community.
> >>
> >> Thanks,
> >> Brian
> >>
> >> On Tue, Sep 1, 2020 at 4:20 PM Brian Neradt <br...@gmail.com>
> >> wrote:
> >>
> >> > This sounds good to me. This essentially puts the type parameter in
> the
> >> > structure itself rather than as a separate parameter to the functions.
> >> >
> >> > On Tue, Sep 1, 2020 at 4:11 PM Alan Carroll
> >> > <so...@verizonmedia.com.invalid> wrote:
> >> >
> >> >> Sorry for chiming in late -
> >> >>
> >> >> Note this is extremely similar to IP addresses and I recommend we use
> >> the
> >> >> same solution. That is, there is a class HttpPriority which has just
> a
> >> >> type/style/family value and possibly a length. This is an abstract
> >> class
> >> >> like sockaddr (which no one actually instantiates). The family
> >> indicates
> >> >> the concrete type, which is something like Http2Priority or
> >> Http3Priority,
> >> >> etc., just like sockaddr_in and sockaddr_in6. When you call the
> >> priority
> >> >> getter, it returns a HttpPriority* which is a pointer to a concrete
> >> >> instance, the type of which is determined by the family. Again, just
> >> like
> >> >> getting an IP address from a socket.
> >> >>
> >> >
> >> >
> >> > --
> >> > "Come to Me, all who are weary and heavy-laden, and I will
> >> > give you rest. Take My yoke upon you and learn from Me, for
> >> > I am gentle and humble in heart, and you will find rest for
> >> > your souls. For My yoke is easy and My burden is light."
> >> >
> >> >     ~ Matthew 11:28-30
> >> >
> >>
> >>
> >> --
> >> "Come to Me, all who are weary and heavy-laden, and I will
> >> give you rest. Take My yoke upon you and learn from Me, for
> >> I am gentle and humble in heart, and you will find rest for
> >> your souls. For My yoke is easy and My burden is light."
> >>
> >>     ~ Matthew 11:28-30
> >>
> >
>

Re: [E] Re: TS API Review: New HTTP/2 stream id and priority getters

Posted by Masakazu Kitajo <ma...@apache.org>.
Actually, I don't understand TSHttpProtocolType. Shouldn't it be
StructureType? Let's say we support a new priority scheme that is available
on both H2 and H3. What would be the TSHttpProtocolType for it?

Masakazu

On Wed, Sep 2, 2020 at 9:37 AM Masakazu Kitajo <ma...@apache.org> wrote:

> +1 I'm ok with the new function signatures that use abstract structures.
>
> However, I'm not sure if we really need that flexibility for Stream ID, I
> can't say 64-bit is big enough, but I think extensibility is not
> everything. Usability matters. Should we prepare for 128-bit stream ids
> now? Would we take the same approach for TSUrlPortGet to prepare for 32-bit
> port numbers?
>
> Masakazu
>
> On Wed, Sep 2, 2020 at 7:35 AM Brian Neradt <br...@gmail.com>
> wrote:
>
>> I updated the protocol draft with this input (notice I kept the updated
>> API
>> implementation as the second commit in the PR for the sake of comparison):
>> https://github.com/apache/trafficserver/pull/7149
>>
>> The API now looks like the following:
>>
>> tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp,
>> TSHttpStreamId *stream_id);
>> tsapi TSReturnCode TSHttpTxnClientStreamPriorityGet(TSHttpTxn txnp,
>> TSHttpPriority *priority);
>>
>> TSHttpStreamId and TSHttpPriority are now abstract structures that can
>> reference TSHttp2StreamId and TSHttp2Priority objects. In the future we
>> can
>> expand this to include version 3 values of these. Notice that the above
>> are
>> generic to the specific HTTP protocol type used by the transaction.
>>
>> I'll hold off on updating the docs in the PR until I get confirmation that
>> this looks OK to the community.
>>
>> Thanks,
>> Brian
>>
>> On Tue, Sep 1, 2020 at 4:20 PM Brian Neradt <br...@gmail.com>
>> wrote:
>>
>> > This sounds good to me. This essentially puts the type parameter in the
>> > structure itself rather than as a separate parameter to the functions.
>> >
>> > On Tue, Sep 1, 2020 at 4:11 PM Alan Carroll
>> > <so...@verizonmedia.com.invalid> wrote:
>> >
>> >> Sorry for chiming in late -
>> >>
>> >> Note this is extremely similar to IP addresses and I recommend we use
>> the
>> >> same solution. That is, there is a class HttpPriority which has just a
>> >> type/style/family value and possibly a length. This is an abstract
>> class
>> >> like sockaddr (which no one actually instantiates). The family
>> indicates
>> >> the concrete type, which is something like Http2Priority or
>> Http3Priority,
>> >> etc., just like sockaddr_in and sockaddr_in6. When you call the
>> priority
>> >> getter, it returns a HttpPriority* which is a pointer to a concrete
>> >> instance, the type of which is determined by the family. Again, just
>> like
>> >> getting an IP address from a socket.
>> >>
>> >
>> >
>> > --
>> > "Come to Me, all who are weary and heavy-laden, and I will
>> > give you rest. Take My yoke upon you and learn from Me, for
>> > I am gentle and humble in heart, and you will find rest for
>> > your souls. For My yoke is easy and My burden is light."
>> >
>> >     ~ Matthew 11:28-30
>> >
>>
>>
>> --
>> "Come to Me, all who are weary and heavy-laden, and I will
>> give you rest. Take My yoke upon you and learn from Me, for
>> I am gentle and humble in heart, and you will find rest for
>> your souls. For My yoke is easy and My burden is light."
>>
>>     ~ Matthew 11:28-30
>>
>

Re: [E] Re: TS API Review: New HTTP/2 stream id and priority getters

Posted by Masakazu Kitajo <ma...@apache.org>.
+1 I'm ok with the new function signatures that use abstract structures.

However, I'm not sure if we really need that flexibility for Stream ID, I
can't say 64-bit is big enough, but I think extensibility is not
everything. Usability matters. Should we prepare for 128-bit stream ids
now? Would we take the same approach for TSUrlPortGet to prepare for 32-bit
port numbers?

Masakazu

On Wed, Sep 2, 2020 at 7:35 AM Brian Neradt <br...@gmail.com> wrote:

> I updated the protocol draft with this input (notice I kept the updated API
> implementation as the second commit in the PR for the sake of comparison):
> https://github.com/apache/trafficserver/pull/7149
>
> The API now looks like the following:
>
> tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp,
> TSHttpStreamId *stream_id);
> tsapi TSReturnCode TSHttpTxnClientStreamPriorityGet(TSHttpTxn txnp,
> TSHttpPriority *priority);
>
> TSHttpStreamId and TSHttpPriority are now abstract structures that can
> reference TSHttp2StreamId and TSHttp2Priority objects. In the future we can
> expand this to include version 3 values of these. Notice that the above are
> generic to the specific HTTP protocol type used by the transaction.
>
> I'll hold off on updating the docs in the PR until I get confirmation that
> this looks OK to the community.
>
> Thanks,
> Brian
>
> On Tue, Sep 1, 2020 at 4:20 PM Brian Neradt <br...@gmail.com>
> wrote:
>
> > This sounds good to me. This essentially puts the type parameter in the
> > structure itself rather than as a separate parameter to the functions.
> >
> > On Tue, Sep 1, 2020 at 4:11 PM Alan Carroll
> > <so...@verizonmedia.com.invalid> wrote:
> >
> >> Sorry for chiming in late -
> >>
> >> Note this is extremely similar to IP addresses and I recommend we use
> the
> >> same solution. That is, there is a class HttpPriority which has just a
> >> type/style/family value and possibly a length. This is an abstract class
> >> like sockaddr (which no one actually instantiates). The family indicates
> >> the concrete type, which is something like Http2Priority or
> Http3Priority,
> >> etc., just like sockaddr_in and sockaddr_in6. When you call the priority
> >> getter, it returns a HttpPriority* which is a pointer to a concrete
> >> instance, the type of which is determined by the family. Again, just
> like
> >> getting an IP address from a socket.
> >>
> >
> >
> > --
> > "Come to Me, all who are weary and heavy-laden, and I will
> > give you rest. Take My yoke upon you and learn from Me, for
> > I am gentle and humble in heart, and you will find rest for
> > your souls. For My yoke is easy and My burden is light."
> >
> >     ~ Matthew 11:28-30
> >
>
>
> --
> "Come to Me, all who are weary and heavy-laden, and I will
> give you rest. Take My yoke upon you and learn from Me, for
> I am gentle and humble in heart, and you will find rest for
> your souls. For My yoke is easy and My burden is light."
>
>     ~ Matthew 11:28-30
>

Re: [E] Re: TS API Review: New HTTP/2 stream id and priority getters

Posted by Brian Neradt <br...@gmail.com>.
I updated the protocol draft with this input (notice I kept the updated API
implementation as the second commit in the PR for the sake of comparison):
https://github.com/apache/trafficserver/pull/7149

The API now looks like the following:

tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp,
TSHttpStreamId *stream_id);
tsapi TSReturnCode TSHttpTxnClientStreamPriorityGet(TSHttpTxn txnp,
TSHttpPriority *priority);

TSHttpStreamId and TSHttpPriority are now abstract structures that can
reference TSHttp2StreamId and TSHttp2Priority objects. In the future we can
expand this to include version 3 values of these. Notice that the above are
generic to the specific HTTP protocol type used by the transaction.

I'll hold off on updating the docs in the PR until I get confirmation that
this looks OK to the community.

Thanks,
Brian

On Tue, Sep 1, 2020 at 4:20 PM Brian Neradt <br...@gmail.com> wrote:

> This sounds good to me. This essentially puts the type parameter in the
> structure itself rather than as a separate parameter to the functions.
>
> On Tue, Sep 1, 2020 at 4:11 PM Alan Carroll
> <so...@verizonmedia.com.invalid> wrote:
>
>> Sorry for chiming in late -
>>
>> Note this is extremely similar to IP addresses and I recommend we use the
>> same solution. That is, there is a class HttpPriority which has just a
>> type/style/family value and possibly a length. This is an abstract class
>> like sockaddr (which no one actually instantiates). The family indicates
>> the concrete type, which is something like Http2Priority or Http3Priority,
>> etc., just like sockaddr_in and sockaddr_in6. When you call the priority
>> getter, it returns a HttpPriority* which is a pointer to a concrete
>> instance, the type of which is determined by the family. Again, just like
>> getting an IP address from a socket.
>>
>
>
> --
> "Come to Me, all who are weary and heavy-laden, and I will
> give you rest. Take My yoke upon you and learn from Me, for
> I am gentle and humble in heart, and you will find rest for
> your souls. For My yoke is easy and My burden is light."
>
>     ~ Matthew 11:28-30
>


-- 
"Come to Me, all who are weary and heavy-laden, and I will
give you rest. Take My yoke upon you and learn from Me, for
I am gentle and humble in heart, and you will find rest for
your souls. For My yoke is easy and My burden is light."

    ~ Matthew 11:28-30

Re: [E] Re: TS API Review: New HTTP/2 stream id and priority getters

Posted by Brian Neradt <br...@gmail.com>.
This sounds good to me. This essentially puts the type parameter in the
structure itself rather than as a separate parameter to the functions.

On Tue, Sep 1, 2020 at 4:11 PM Alan Carroll
<so...@verizonmedia.com.invalid> wrote:

> Sorry for chiming in late -
>
> Note this is extremely similar to IP addresses and I recommend we use the
> same solution. That is, there is a class HttpPriority which has just a
> type/style/family value and possibly a length. This is an abstract class
> like sockaddr (which no one actually instantiates). The family indicates
> the concrete type, which is something like Http2Priority or Http3Priority,
> etc., just like sockaddr_in and sockaddr_in6. When you call the priority
> getter, it returns a HttpPriority* which is a pointer to a concrete
> instance, the type of which is determined by the family. Again, just like
> getting an IP address from a socket.
>


-- 
"Come to Me, all who are weary and heavy-laden, and I will
give you rest. Take My yoke upon you and learn from Me, for
I am gentle and humble in heart, and you will find rest for
your souls. For My yoke is easy and My burden is light."

    ~ Matthew 11:28-30

Re: [E] TS API Review: New HTTP/2 stream id and priority getters

Posted by Brian Neradt <br...@gmail.com>.
Considering this input:

   - I changed the TSHttpTxnClientStreamIdGet to return a uint64_t.
   - I removed 0_9 from the protocol type list and added
   HTTP_PROTOCOL_TYPE_HTTP_UNSPECIFIED.
   - I renamed TSHttpProtocolType to TSHttpPriorityType.

The API functions now look like:

tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp, uint64_t
*stream_id);
tsapi TSReturnCode TSHttpTxnClientStreamPriorityGet(TSHttpTxn txnp,
TSHttpPriority *priority);

The draft PR is updated with these changes:
https://github.com/apache/trafficserver/pull/7149

Thanks,

On Wed, Sep 2, 2020 at 12:37 PM Brian Neradt <br...@gmail.com> wrote:

> Correct, a regular C struct.
>
> On Wed, Sep 2, 2020 at 11:28 AM Leif Hedstrom <zw...@apache.org> wrote:
>
>>
>>
>> > On Sep 1, 2020, at 3:11 PM, Alan Carroll
>> <so...@verizonmedia.com.INVALID> wrote:
>> >
>> > Sorry for chiming in late -
>> >
>> > Note this is extremely similar to IP addresses and I recommend we use
>> the
>> > same solution. That is, there is a class HttpPriority which has just a
>> > type/style/family value and possibly a length. This is an abstract class
>> > like sockaddr (which no one actually instantiates). The family indicates
>> > the concrete type, which is something like Http2Priority or
>> Http3Priority,
>> > etc., just like sockaddr_in and sockaddr_in6. When you call the priority
>> > getter, it returns a HttpPriority* which is a pointer to a concrete
>> > instance, the type of which is determined by the family. Again, just
>> like
>> > getting an IP address from a socket.
>>
>>
>> I assume with class here, you mean a regular C struct ?
>>
>> — Leif
>>
>>
>>
>
> --
> "Come to Me, all who are weary and heavy-laden, and I will
> give you rest. Take My yoke upon you and learn from Me, for
> I am gentle and humble in heart, and you will find rest for
> your souls. For My yoke is easy and My burden is light."
>
>     ~ Matthew 11:28-30
>


-- 
"Come to Me, all who are weary and heavy-laden, and I will
give you rest. Take My yoke upon you and learn from Me, for
I am gentle and humble in heart, and you will find rest for
your souls. For My yoke is easy and My burden is light."

    ~ Matthew 11:28-30

Re: [E] TS API Review: New HTTP/2 stream id and priority getters

Posted by Brian Neradt <br...@gmail.com>.
Correct, a regular C struct.

On Wed, Sep 2, 2020 at 11:28 AM Leif Hedstrom <zw...@apache.org> wrote:

>
>
> > On Sep 1, 2020, at 3:11 PM, Alan Carroll
> <so...@verizonmedia.com.INVALID> wrote:
> >
> > Sorry for chiming in late -
> >
> > Note this is extremely similar to IP addresses and I recommend we use the
> > same solution. That is, there is a class HttpPriority which has just a
> > type/style/family value and possibly a length. This is an abstract class
> > like sockaddr (which no one actually instantiates). The family indicates
> > the concrete type, which is something like Http2Priority or
> Http3Priority,
> > etc., just like sockaddr_in and sockaddr_in6. When you call the priority
> > getter, it returns a HttpPriority* which is a pointer to a concrete
> > instance, the type of which is determined by the family. Again, just like
> > getting an IP address from a socket.
>
>
> I assume with class here, you mean a regular C struct ?
>
> — Leif
>
>
>

-- 
"Come to Me, all who are weary and heavy-laden, and I will
give you rest. Take My yoke upon you and learn from Me, for
I am gentle and humble in heart, and you will find rest for
your souls. For My yoke is easy and My burden is light."

    ~ Matthew 11:28-30

Re: [E] TS API Review: New HTTP/2 stream id and priority getters

Posted by Leif Hedstrom <zw...@apache.org>.

> On Sep 1, 2020, at 3:11 PM, Alan Carroll <so...@verizonmedia.com.INVALID> wrote:
> 
> Sorry for chiming in late -
> 
> Note this is extremely similar to IP addresses and I recommend we use the
> same solution. That is, there is a class HttpPriority which has just a
> type/style/family value and possibly a length. This is an abstract class
> like sockaddr (which no one actually instantiates). The family indicates
> the concrete type, which is something like Http2Priority or Http3Priority,
> etc., just like sockaddr_in and sockaddr_in6. When you call the priority
> getter, it returns a HttpPriority* which is a pointer to a concrete
> instance, the type of which is determined by the family. Again, just like
> getting an IP address from a socket.


I assume with class here, you mean a regular C struct ?

— Leif



Re: [E] Re: TS API Review: New HTTP/2 stream id and priority getters

Posted by Alan Carroll <so...@verizonmedia.com.INVALID>.
Sorry for chiming in late -

Note this is extremely similar to IP addresses and I recommend we use the
same solution. That is, there is a class HttpPriority which has just a
type/style/family value and possibly a length. This is an abstract class
like sockaddr (which no one actually instantiates). The family indicates
the concrete type, which is something like Http2Priority or Http3Priority,
etc., just like sockaddr_in and sockaddr_in6. When you call the priority
getter, it returns a HttpPriority* which is a pointer to a concrete
instance, the type of which is determined by the family. Again, just like
getting an IP address from a socket.

Re: TS API Review: New HTTP/2 stream id and priority getters

Posted by Brian Neradt <br...@gmail.com>.
Thank you Sudheer, this is helpful.

Considering Masaori's input, it sounds like it would be wise to amend
TSHttpTxnClientPriorityGet to just take a single, generic priority opaque
structure:

 tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp,
TSHttpProtocolType type, void *stream_id);
 tsapi TSReturnCode TSHttpTxnClientPriorityGet(TSHttpTxn
txnp, TSHttpProtocolType type, void *priority);

I'll work on a prototype of this and post it as a separate commit on the
draft PR.

On Tue, Sep 1, 2020 at 10:52 AM Sudheer Vinukonda
<su...@yahoo.com.invalid> wrote:

>  >> (primary reason for that being let the user have "control" on what
> they want to retrieve).
> And of course, the API shall return TS_ERROR, if the passed in protocol
> "type" is not correct for the given TXN.
>     On Tuesday, September 1, 2020, 08:45:21 AM PDT, Sudheer Vinukonda <
> sudheervinukonda@yahoo.com.invalid> wrote:
>
>   +1 to use opaque structures and use protocol "type" as the "key".
> I'd slightly prefer the caller pass in the protocol type rather than
> letting TS automatically determine that (primary reason for that being let
> the user have "control" on what they want to retrieve).
> For e.g
>
>  tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp,
> TSHttpProtocolType type, void *stream_id);
>  tsapi TSReturnCode TSHttpTxnClientPriorityGet(TSHttpTxn
> txnp, TSHttpProtocolType type, void *stream_dependency, void *weight);
>
> where, `TSHttpProtocolType` is a new interface we add with applicable HTTP
> protocols defined and it is up to the caller to pass in appropriate data
> types for any given protocol they call the API for.
>
> All that said, like Leif, I don't feel super strong on this as well
> and, I'd be okay with the general consensus.
>
> Thanks,
> Sudheer
>
>
>
>     On Tuesday, September 1, 2020, 08:02:16 AM PDT, Leif Hedstrom <
> zwoop@apache.org> wrote:
>
>
>
> > On Sep 1, 2020, at 01:20, Masaori Koshiba <ma...@apache.org> wrote:
> >
> > I totally agree with less API is better.
> >
> >> tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp,
> > uint64_t *stream_id);
> >
> > Stripping "Http2" looks reasonable. We can use this API for protocols
> that
> > have stream id.
> >
> >> tsapi TSReturnCode TSHttpTxnClientPriorityGet(TSHttpTxn txnp,
> > uint64_t *stream_dependency, int *weight);
> >
> > I doubt removing "Http2" from this API makes sense. Because this API is
> > assuming HTTP/2's priority scheme (dependency & weight).
> > As Masakazu pointed out, a new priority scheme is proposed and it's
> > completely different from HTTP/2's priority scheme.
> > It's trying to make a priority scheme HTTP version-independent.
>
> Rather than making the API protocol specific, why not use opaque
> structures, and a protocol “type” identifier returned? Even with H2 and
> later H3, it’s possible things like priority keeps changing.
>
> The caller would of course have to type cast to the appropriate priority
> struct. However,  I don’t feel super strongly here, and sadly we decided
> not to turn the public APIs into C++ where this type of behavior is more
> natural.
>
> — Leif
> >
> > So I'd like to leave "Http2" for this API and reserve
> > "TSHttpTxnClientPriorityGet" for the new priority scheme.
> >
> > - Masaori
> >
> >> On Tue, Sep 1, 2020 at 12:02 PM Brian Neradt <br...@gmail.com>
> wrote:
> >>
> >> Thanks all, this is good feedback. It sounds like we're in agreement on
> >> dropping the explicit reference to Http2 so this can accommodate future
> >> protocols. Considering this and the observation that stream_id is 62
> bits
> >> for HTTP/3, the output parameters should be increased in size
> accordingly.
> >> That would make the API look like the following:
> >>
> >> tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp, uint64_t
> >> *stream_id);
> >>
> >> tsapi TSReturnCode TSHttpTxnClientPriorityGet(TSHttpTxn txnp, uint64_t
> >> *stream_dependency, int *weight);
> >>
> >> This will satisfy the Traffic Dump plugin needs. The concern raised
> about
> >> protocol identification is not an issue for Traffic Dump because it will
> >> know the protocol from either the protocol retrieval API
> >> (TSHttpSsnClientProtocolStackGet) or from the version in the request
> >> via TSHttpHdrVersionGet. That being the case, I'm content not adding a
> >> separate output parameter for this.
> >>
> >> That said, I don't want to dismiss the option too quickly if others are
> >> still interested. Sudheer, you seem to have the most developed ideas for
> >> this. If, considering what I say above, you'd still like us to consider
> the
> >> inclusion of a context parameter, could you please flesh out:
> >>
> >>  - What the signatures for the stream id and priority getters would look
> >>  like.
> >>  - The structure of the context object.
> >>  - Would all HTTP/2 and above APIs contain this context parameter?
> >>
> >> Otherwise, if we're OK not having the context parameter, I can update
> the
> >> draft PR with the above changes to the API.
> >>
> >> Thanks,
> >>
> >> On Mon, Aug 31, 2020 at 9:19 PM Sudheer Vinukonda
> >> <su...@yahoo.com.invalid> wrote:
> >>
> >>> Right, it does make sense to have a generic API naming (and remove
> http2
> >>> reference from it), but I'm thinking it should still allow to pass in
> >> some
> >>> context object (we may define an interface for it, with "protocol"
> being
> >> a
> >>> mandatory field) to be able to continue using it in the future as well
> as
> >>> to let user explicitly specify that they want stream id for http2 or
> >> http2
> >>> for example?
> >>> Thanks,
> >>> Sudheer
> >>>    On Monday, August 31, 2020, 06:49:57 PM PDT, Masakazu Kitajo <
> >>> m4sk17@gmail.com> wrote:
> >>>
> >>> Sudheer,
> >>>
> >>> Would a separated API to get the actual protocol active for the given
> >>> request satisfy you? I don't understand how indicating the actual
> >> protocol
> >>> helps something.
> >>>
> >>> I think I understand your concern. Future protocols may need different
> >>> params, or might have different representations. However, H/2 is
> >> extensible
> >>> so any APIs that have "Http2" in their names still can be incompatible
> if
> >>> we support extensions. The proposal for a new priority scheme is a good
> >>> example.
> >>>
> >>> In that sense, passing a context object may be helpful, and more
> flexible
> >>> than having fixed context in API names, but maybe it's too early to
> think
> >>> about it.
> >>>
> >>> Masakazu
> >>>
> >>>
> >>> On Mon, Aug 31, 2020 at 11:05 AM Sudheer Vinukonda
> >>> <su...@yahoo.com.invalid> wrote:
> >>>
> >>>> Hmm, it does sound appealing to avoid having to duplicate API for
> >> common
> >>>> attributes between H/2 and H/3, but, I'm thinking the API should still
> >>>> somehow explicitly indicate (either via an input param or an output
> >>> param)
> >>>> the actual protocol active for the given request. It seems a bit too
> >>>> confusing (and potentially problematic depending on how things evolve
> >> in
> >>>> the future) to hide that aspect.
> >>>> Thoughts?
> >>>>
> >>>>
> >>>>  On Sunday, August 30, 2020, 06:56:49 PM PDT, Masakazu Kitajo <
> >>>> maskit@apache.org> wrote:
> >>>>
> >>>> And there is a proposal for a new priority scheme that may be used for
> >>>> both
> >>>> H2 and H3.
> >>>> https://tools.ietf.org/html/draft-ietf-httpbis-priority-01
> >>>>
> >>>> I haven't read it, but it would be nice if the TS API could be
> >> compatible
> >>>> with the new scheme.
> >>>>
> >>>> On Mon, Aug 31, 2020 at 10:48 AM Masakazu Kitajo <ma...@apache.org>
> >>>> wrote:
> >>>>
> >>>>> I think we should avoid adding APIs just for HTTP/2 unless it's
> >> really
> >>>>> HTTP/2 specific. Are we going to add TSHttpTxnClientHttp3StreamIdGet
> >>> and
> >>>>> TSHttpTxnClientHttp3PriorityGet as well?
> >>>>>
> >>>>> I'd omit the "2" in the API names, and rephrase the API doc like:
> >>>>> This API returns an error if the provided transaction does not have a
> >>>>> stream id.
> >>>>>
> >>>>> Also, a stream id on HTTP/3 (QUIC) is 62 bit integer.
> >>>>>
> >>>>> Masakazu
> >>>>>
> >>>>>
> >>>>> On Sat, Aug 29, 2020 at 5:24 AM SUSAN HINRICHS <sh...@ieee.org>
> >>>> wrote:
> >>>>>
> >>>>>> +1
> >>>>>>
> >>>>>> On Fri, Aug 28, 2020 at 3:23 PM Sudheer Vinukonda
> >>>>>> <su...@yahoo.com.invalid> wrote:
> >>>>>>
> >>>>>>> +1.
> >>>>>>> Sounds reasonable to me.
> >>>>>>>
> >>>>>>>
> >>>>>>>  On Friday, August 28, 2020, 07:41:18 AM PDT, Brian Neradt <
> >>>>>>> brian.neradt@gmail.com> wrote:
> >>>>>>>
> >>>>>>> Hi dev@trafficserver.apache.org,
> >>>>>>>
> >>>>>>> It would be helpful to add HTTP/2 stream and priority support to
> >>>> Traffic
> >>>>>>> Dump. In order for the plugin to access this information, I
> >> propose
> >>>>>> adding
> >>>>>>> the following two functions to the TS API:
> >>>>>>>
> >>>>>>> tsapi TSReturnCode TSHttpTxnClientHttp2StreamIdGet(TSHttpTxn txnp,
> >>>>>> uint32_t
> >>>>>>> *stream_id);
> >>>>>>>
> >>>>>>> tsapi TSReturnCode TSHttpTxnClientHttp2PriorityGet(TSHttpTxn txnp,
> >>> int
> >>>>>>> *stream_dependency, int *weight);
> >>>>>>>
> >>>>>>> I've created a draft PR with this implemented along with Traffic
> >>>> Dump's
> >>>>>> use
> >>>>>>> of it:
> >>>>>>> https://github.com/apache/trafficserver/pull/7149
> >>>>>>>
> >>>>>>> Please let me know if there are any concerns or suggestions for
> >>>>>> improvement
> >>>>>>> concerning this.
> >>>>>>>
> >>>>>>> Thanks!
> >>>>>>> Brian
> >>>>>>>
> >>>>>>> --
> >>>>>>> "Come to Me, all who are weary and heavy-laden, and I will
> >>>>>>> give you rest. Take My yoke upon you and learn from Me, for
> >>>>>>> I am gentle and humble in heart, and you will find rest for
> >>>>>>> your souls. For My yoke is easy and My burden is light."
> >>>>>>>
> >>>>>>>  ~ Matthew 11:28-30
> >>>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>>
> >>
> >>
> >>
> >> --
> >> "Come to Me, all who are weary and heavy-laden, and I will
> >> give you rest. Take My yoke upon you and learn from Me, for
> >> I am gentle and humble in heart, and you will find rest for
> >> your souls. For My yoke is easy and My burden is light."
> >>
> >>    ~ Matthew 11:28-30
> >>
>



-- 
"Come to Me, all who are weary and heavy-laden, and I will
give you rest. Take My yoke upon you and learn from Me, for
I am gentle and humble in heart, and you will find rest for
your souls. For My yoke is easy and My burden is light."

    ~ Matthew 11:28-30

Re: TS API Review: New HTTP/2 stream id and priority getters

Posted by Sudheer Vinukonda <su...@yahoo.com.INVALID>.
 >> (primary reason for that being let the user have "control" on what they want to retrieve). 
And of course, the API shall return TS_ERROR, if the passed in protocol "type" is not correct for the given TXN.
    On Tuesday, September 1, 2020, 08:45:21 AM PDT, Sudheer Vinukonda <su...@yahoo.com.invalid> wrote:  
 
  +1 to use opaque structures and use protocol "type" as the "key".
I'd slightly prefer the caller pass in the protocol type rather than letting TS automatically determine that (primary reason for that being let the user have "control" on what they want to retrieve). 
For e.g

 tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp, TSHttpProtocolType type, void *stream_id);
 tsapi TSReturnCode TSHttpTxnClientPriorityGet(TSHttpTxn txnp, TSHttpProtocolType type, void *stream_dependency, void *weight);

where, `TSHttpProtocolType` is a new interface we add with applicable HTTP protocols defined and it is up to the caller to pass in appropriate data types for any given protocol they call the API for. 

All that said, like Leif, I don't feel super strong on this as well and, I'd be okay with the general consensus.

Thanks,
Sudheer



    On Tuesday, September 1, 2020, 08:02:16 AM PDT, Leif Hedstrom <zw...@apache.org> wrote:  
 
 

> On Sep 1, 2020, at 01:20, Masaori Koshiba <ma...@apache.org> wrote:
> 
> I totally agree with less API is better.
> 
>> tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp,
> uint64_t *stream_id);
> 
> Stripping "Http2" looks reasonable. We can use this API for protocols that
> have stream id.
> 
>> tsapi TSReturnCode TSHttpTxnClientPriorityGet(TSHttpTxn txnp,
> uint64_t *stream_dependency, int *weight);
> 
> I doubt removing "Http2" from this API makes sense. Because this API is
> assuming HTTP/2's priority scheme (dependency & weight).
> As Masakazu pointed out, a new priority scheme is proposed and it's
> completely different from HTTP/2's priority scheme.
> It's trying to make a priority scheme HTTP version-independent.

Rather than making the API protocol specific, why not use opaque structures, and a protocol “type” identifier returned? Even with H2 and later H3, it’s possible things like priority keeps changing.

The caller would of course have to type cast to the appropriate priority struct. However,  I don’t feel super strongly here, and sadly we decided not to turn the public APIs into C++ where this type of behavior is more natural.

— Leif
> 
> So I'd like to leave "Http2" for this API and reserve
> "TSHttpTxnClientPriorityGet" for the new priority scheme.
> 
> - Masaori
> 
>> On Tue, Sep 1, 2020 at 12:02 PM Brian Neradt <br...@gmail.com> wrote:
>> 
>> Thanks all, this is good feedback. It sounds like we're in agreement on
>> dropping the explicit reference to Http2 so this can accommodate future
>> protocols. Considering this and the observation that stream_id is 62 bits
>> for HTTP/3, the output parameters should be increased in size accordingly.
>> That would make the API look like the following:
>> 
>> tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp, uint64_t
>> *stream_id);
>> 
>> tsapi TSReturnCode TSHttpTxnClientPriorityGet(TSHttpTxn txnp, uint64_t
>> *stream_dependency, int *weight);
>> 
>> This will satisfy the Traffic Dump plugin needs. The concern raised about
>> protocol identification is not an issue for Traffic Dump because it will
>> know the protocol from either the protocol retrieval API
>> (TSHttpSsnClientProtocolStackGet) or from the version in the request
>> via TSHttpHdrVersionGet. That being the case, I'm content not adding a
>> separate output parameter for this.
>> 
>> That said, I don't want to dismiss the option too quickly if others are
>> still interested. Sudheer, you seem to have the most developed ideas for
>> this. If, considering what I say above, you'd still like us to consider the
>> inclusion of a context parameter, could you please flesh out:
>> 
>>  - What the signatures for the stream id and priority getters would look
>>  like.
>>  - The structure of the context object.
>>  - Would all HTTP/2 and above APIs contain this context parameter?
>> 
>> Otherwise, if we're OK not having the context parameter, I can update the
>> draft PR with the above changes to the API.
>> 
>> Thanks,
>> 
>> On Mon, Aug 31, 2020 at 9:19 PM Sudheer Vinukonda
>> <su...@yahoo.com.invalid> wrote:
>> 
>>> Right, it does make sense to have a generic API naming (and remove http2
>>> reference from it), but I'm thinking it should still allow to pass in
>> some
>>> context object (we may define an interface for it, with "protocol" being
>> a
>>> mandatory field) to be able to continue using it in the future as well as
>>> to let user explicitly specify that they want stream id for http2 or
>> http2
>>> for example?
>>> Thanks,
>>> Sudheer
>>>    On Monday, August 31, 2020, 06:49:57 PM PDT, Masakazu Kitajo <
>>> m4sk17@gmail.com> wrote:
>>> 
>>> Sudheer,
>>> 
>>> Would a separated API to get the actual protocol active for the given
>>> request satisfy you? I don't understand how indicating the actual
>> protocol
>>> helps something.
>>> 
>>> I think I understand your concern. Future protocols may need different
>>> params, or might have different representations. However, H/2 is
>> extensible
>>> so any APIs that have "Http2" in their names still can be incompatible if
>>> we support extensions. The proposal for a new priority scheme is a good
>>> example.
>>> 
>>> In that sense, passing a context object may be helpful, and more flexible
>>> than having fixed context in API names, but maybe it's too early to think
>>> about it.
>>> 
>>> Masakazu
>>> 
>>> 
>>> On Mon, Aug 31, 2020 at 11:05 AM Sudheer Vinukonda
>>> <su...@yahoo.com.invalid> wrote:
>>> 
>>>> Hmm, it does sound appealing to avoid having to duplicate API for
>> common
>>>> attributes between H/2 and H/3, but, I'm thinking the API should still
>>>> somehow explicitly indicate (either via an input param or an output
>>> param)
>>>> the actual protocol active for the given request. It seems a bit too
>>>> confusing (and potentially problematic depending on how things evolve
>> in
>>>> the future) to hide that aspect.
>>>> Thoughts?
>>>> 
>>>> 
>>>>  On Sunday, August 30, 2020, 06:56:49 PM PDT, Masakazu Kitajo <
>>>> maskit@apache.org> wrote:
>>>> 
>>>> And there is a proposal for a new priority scheme that may be used for
>>>> both
>>>> H2 and H3.
>>>> https://tools.ietf.org/html/draft-ietf-httpbis-priority-01
>>>> 
>>>> I haven't read it, but it would be nice if the TS API could be
>> compatible
>>>> with the new scheme.
>>>> 
>>>> On Mon, Aug 31, 2020 at 10:48 AM Masakazu Kitajo <ma...@apache.org>
>>>> wrote:
>>>> 
>>>>> I think we should avoid adding APIs just for HTTP/2 unless it's
>> really
>>>>> HTTP/2 specific. Are we going to add TSHttpTxnClientHttp3StreamIdGet
>>> and
>>>>> TSHttpTxnClientHttp3PriorityGet as well?
>>>>> 
>>>>> I'd omit the "2" in the API names, and rephrase the API doc like:
>>>>> This API returns an error if the provided transaction does not have a
>>>>> stream id.
>>>>> 
>>>>> Also, a stream id on HTTP/3 (QUIC) is 62 bit integer.
>>>>> 
>>>>> Masakazu
>>>>> 
>>>>> 
>>>>> On Sat, Aug 29, 2020 at 5:24 AM SUSAN HINRICHS <sh...@ieee.org>
>>>> wrote:
>>>>> 
>>>>>> +1
>>>>>> 
>>>>>> On Fri, Aug 28, 2020 at 3:23 PM Sudheer Vinukonda
>>>>>> <su...@yahoo.com.invalid> wrote:
>>>>>> 
>>>>>>> +1.
>>>>>>> Sounds reasonable to me.
>>>>>>> 
>>>>>>> 
>>>>>>>  On Friday, August 28, 2020, 07:41:18 AM PDT, Brian Neradt <
>>>>>>> brian.neradt@gmail.com> wrote:
>>>>>>> 
>>>>>>> Hi dev@trafficserver.apache.org,
>>>>>>> 
>>>>>>> It would be helpful to add HTTP/2 stream and priority support to
>>>> Traffic
>>>>>>> Dump. In order for the plugin to access this information, I
>> propose
>>>>>> adding
>>>>>>> the following two functions to the TS API:
>>>>>>> 
>>>>>>> tsapi TSReturnCode TSHttpTxnClientHttp2StreamIdGet(TSHttpTxn txnp,
>>>>>> uint32_t
>>>>>>> *stream_id);
>>>>>>> 
>>>>>>> tsapi TSReturnCode TSHttpTxnClientHttp2PriorityGet(TSHttpTxn txnp,
>>> int
>>>>>>> *stream_dependency, int *weight);
>>>>>>> 
>>>>>>> I've created a draft PR with this implemented along with Traffic
>>>> Dump's
>>>>>> use
>>>>>>> of it:
>>>>>>> https://github.com/apache/trafficserver/pull/7149
>>>>>>> 
>>>>>>> Please let me know if there are any concerns or suggestions for
>>>>>> improvement
>>>>>>> concerning this.
>>>>>>> 
>>>>>>> Thanks!
>>>>>>> Brian
>>>>>>> 
>>>>>>> --
>>>>>>> "Come to Me, all who are weary and heavy-laden, and I will
>>>>>>> give you rest. Take My yoke upon you and learn from Me, for
>>>>>>> I am gentle and humble in heart, and you will find rest for
>>>>>>> your souls. For My yoke is easy and My burden is light."
>>>>>>> 
>>>>>>>  ~ Matthew 11:28-30
>>>>>>> 
>>>>>> 
>>>>> 
>>>> 
>>> 
>> 
>> 
>> 
>> --
>> "Come to Me, all who are weary and heavy-laden, and I will
>> give you rest. Take My yoke upon you and learn from Me, for
>> I am gentle and humble in heart, and you will find rest for
>> your souls. For My yoke is easy and My burden is light."
>> 
>>    ~ Matthew 11:28-30
>> 
    

Re: TS API Review: New HTTP/2 stream id and priority getters

Posted by Sudheer Vinukonda <su...@yahoo.com.INVALID>.
 +1 to use opaque structures and use protocol "type" as the "key".
I'd slightly prefer the caller pass in the protocol type rather than letting TS automatically determine that (primary reason for that being let the user have "control" on what they want to retrieve). 
For e.g

 tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp, TSHttpProtocolType type, void *stream_id);
 tsapi TSReturnCode TSHttpTxnClientPriorityGet(TSHttpTxn txnp, TSHttpProtocolType type, void *stream_dependency, void *weight);

where, `TSHttpProtocolType` is a new interface we add with applicable HTTP protocols defined and it is up to the caller to pass in appropriate data types for any given protocol they call the API for. 

All that said, like Leif, I don't feel super strong on this as well and, I'd be okay with the general consensus.

Thanks,
Sudheer



    On Tuesday, September 1, 2020, 08:02:16 AM PDT, Leif Hedstrom <zw...@apache.org> wrote:  
 
 

> On Sep 1, 2020, at 01:20, Masaori Koshiba <ma...@apache.org> wrote:
> 
> I totally agree with less API is better.
> 
>> tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp,
> uint64_t *stream_id);
> 
> Stripping "Http2" looks reasonable. We can use this API for protocols that
> have stream id.
> 
>> tsapi TSReturnCode TSHttpTxnClientPriorityGet(TSHttpTxn txnp,
> uint64_t *stream_dependency, int *weight);
> 
> I doubt removing "Http2" from this API makes sense. Because this API is
> assuming HTTP/2's priority scheme (dependency & weight).
> As Masakazu pointed out, a new priority scheme is proposed and it's
> completely different from HTTP/2's priority scheme.
> It's trying to make a priority scheme HTTP version-independent.

Rather than making the API protocol specific, why not use opaque structures, and a protocol “type” identifier returned? Even with H2 and later H3, it’s possible things like priority keeps changing.

The caller would of course have to type cast to the appropriate priority struct. However,  I don’t feel super strongly here, and sadly we decided not to turn the public APIs into C++ where this type of behavior is more natural.

— Leif
> 
> So I'd like to leave "Http2" for this API and reserve
> "TSHttpTxnClientPriorityGet" for the new priority scheme.
> 
> - Masaori
> 
>> On Tue, Sep 1, 2020 at 12:02 PM Brian Neradt <br...@gmail.com> wrote:
>> 
>> Thanks all, this is good feedback. It sounds like we're in agreement on
>> dropping the explicit reference to Http2 so this can accommodate future
>> protocols. Considering this and the observation that stream_id is 62 bits
>> for HTTP/3, the output parameters should be increased in size accordingly.
>> That would make the API look like the following:
>> 
>> tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp, uint64_t
>> *stream_id);
>> 
>> tsapi TSReturnCode TSHttpTxnClientPriorityGet(TSHttpTxn txnp, uint64_t
>> *stream_dependency, int *weight);
>> 
>> This will satisfy the Traffic Dump plugin needs. The concern raised about
>> protocol identification is not an issue for Traffic Dump because it will
>> know the protocol from either the protocol retrieval API
>> (TSHttpSsnClientProtocolStackGet) or from the version in the request
>> via TSHttpHdrVersionGet. That being the case, I'm content not adding a
>> separate output parameter for this.
>> 
>> That said, I don't want to dismiss the option too quickly if others are
>> still interested. Sudheer, you seem to have the most developed ideas for
>> this. If, considering what I say above, you'd still like us to consider the
>> inclusion of a context parameter, could you please flesh out:
>> 
>>  - What the signatures for the stream id and priority getters would look
>>  like.
>>  - The structure of the context object.
>>  - Would all HTTP/2 and above APIs contain this context parameter?
>> 
>> Otherwise, if we're OK not having the context parameter, I can update the
>> draft PR with the above changes to the API.
>> 
>> Thanks,
>> 
>> On Mon, Aug 31, 2020 at 9:19 PM Sudheer Vinukonda
>> <su...@yahoo.com.invalid> wrote:
>> 
>>> Right, it does make sense to have a generic API naming (and remove http2
>>> reference from it), but I'm thinking it should still allow to pass in
>> some
>>> context object (we may define an interface for it, with "protocol" being
>> a
>>> mandatory field) to be able to continue using it in the future as well as
>>> to let user explicitly specify that they want stream id for http2 or
>> http2
>>> for example?
>>> Thanks,
>>> Sudheer
>>>    On Monday, August 31, 2020, 06:49:57 PM PDT, Masakazu Kitajo <
>>> m4sk17@gmail.com> wrote:
>>> 
>>> Sudheer,
>>> 
>>> Would a separated API to get the actual protocol active for the given
>>> request satisfy you? I don't understand how indicating the actual
>> protocol
>>> helps something.
>>> 
>>> I think I understand your concern. Future protocols may need different
>>> params, or might have different representations. However, H/2 is
>> extensible
>>> so any APIs that have "Http2" in their names still can be incompatible if
>>> we support extensions. The proposal for a new priority scheme is a good
>>> example.
>>> 
>>> In that sense, passing a context object may be helpful, and more flexible
>>> than having fixed context in API names, but maybe it's too early to think
>>> about it.
>>> 
>>> Masakazu
>>> 
>>> 
>>> On Mon, Aug 31, 2020 at 11:05 AM Sudheer Vinukonda
>>> <su...@yahoo.com.invalid> wrote:
>>> 
>>>> Hmm, it does sound appealing to avoid having to duplicate API for
>> common
>>>> attributes between H/2 and H/3, but, I'm thinking the API should still
>>>> somehow explicitly indicate (either via an input param or an output
>>> param)
>>>> the actual protocol active for the given request. It seems a bit too
>>>> confusing (and potentially problematic depending on how things evolve
>> in
>>>> the future) to hide that aspect.
>>>> Thoughts?
>>>> 
>>>> 
>>>>  On Sunday, August 30, 2020, 06:56:49 PM PDT, Masakazu Kitajo <
>>>> maskit@apache.org> wrote:
>>>> 
>>>> And there is a proposal for a new priority scheme that may be used for
>>>> both
>>>> H2 and H3.
>>>> https://tools.ietf.org/html/draft-ietf-httpbis-priority-01
>>>> 
>>>> I haven't read it, but it would be nice if the TS API could be
>> compatible
>>>> with the new scheme.
>>>> 
>>>> On Mon, Aug 31, 2020 at 10:48 AM Masakazu Kitajo <ma...@apache.org>
>>>> wrote:
>>>> 
>>>>> I think we should avoid adding APIs just for HTTP/2 unless it's
>> really
>>>>> HTTP/2 specific. Are we going to add TSHttpTxnClientHttp3StreamIdGet
>>> and
>>>>> TSHttpTxnClientHttp3PriorityGet as well?
>>>>> 
>>>>> I'd omit the "2" in the API names, and rephrase the API doc like:
>>>>> This API returns an error if the provided transaction does not have a
>>>>> stream id.
>>>>> 
>>>>> Also, a stream id on HTTP/3 (QUIC) is 62 bit integer.
>>>>> 
>>>>> Masakazu
>>>>> 
>>>>> 
>>>>> On Sat, Aug 29, 2020 at 5:24 AM SUSAN HINRICHS <sh...@ieee.org>
>>>> wrote:
>>>>> 
>>>>>> +1
>>>>>> 
>>>>>> On Fri, Aug 28, 2020 at 3:23 PM Sudheer Vinukonda
>>>>>> <su...@yahoo.com.invalid> wrote:
>>>>>> 
>>>>>>> +1.
>>>>>>> Sounds reasonable to me.
>>>>>>> 
>>>>>>> 
>>>>>>>  On Friday, August 28, 2020, 07:41:18 AM PDT, Brian Neradt <
>>>>>>> brian.neradt@gmail.com> wrote:
>>>>>>> 
>>>>>>> Hi dev@trafficserver.apache.org,
>>>>>>> 
>>>>>>> It would be helpful to add HTTP/2 stream and priority support to
>>>> Traffic
>>>>>>> Dump. In order for the plugin to access this information, I
>> propose
>>>>>> adding
>>>>>>> the following two functions to the TS API:
>>>>>>> 
>>>>>>> tsapi TSReturnCode TSHttpTxnClientHttp2StreamIdGet(TSHttpTxn txnp,
>>>>>> uint32_t
>>>>>>> *stream_id);
>>>>>>> 
>>>>>>> tsapi TSReturnCode TSHttpTxnClientHttp2PriorityGet(TSHttpTxn txnp,
>>> int
>>>>>>> *stream_dependency, int *weight);
>>>>>>> 
>>>>>>> I've created a draft PR with this implemented along with Traffic
>>>> Dump's
>>>>>> use
>>>>>>> of it:
>>>>>>> https://github.com/apache/trafficserver/pull/7149
>>>>>>> 
>>>>>>> Please let me know if there are any concerns or suggestions for
>>>>>> improvement
>>>>>>> concerning this.
>>>>>>> 
>>>>>>> Thanks!
>>>>>>> Brian
>>>>>>> 
>>>>>>> --
>>>>>>> "Come to Me, all who are weary and heavy-laden, and I will
>>>>>>> give you rest. Take My yoke upon you and learn from Me, for
>>>>>>> I am gentle and humble in heart, and you will find rest for
>>>>>>> your souls. For My yoke is easy and My burden is light."
>>>>>>> 
>>>>>>>  ~ Matthew 11:28-30
>>>>>>> 
>>>>>> 
>>>>> 
>>>> 
>>> 
>> 
>> 
>> 
>> --
>> "Come to Me, all who are weary and heavy-laden, and I will
>> give you rest. Take My yoke upon you and learn from Me, for
>> I am gentle and humble in heart, and you will find rest for
>> your souls. For My yoke is easy and My burden is light."
>> 
>>    ~ Matthew 11:28-30
>> 
  

Re: TS API Review: New HTTP/2 stream id and priority getters

Posted by Leif Hedstrom <zw...@apache.org>.

> On Sep 1, 2020, at 01:20, Masaori Koshiba <ma...@apache.org> wrote:
> 
> I totally agree with less API is better.
> 
>> tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp,
> uint64_t *stream_id);
> 
> Stripping "Http2" looks reasonable. We can use this API for protocols that
> have stream id.
> 
>> tsapi TSReturnCode TSHttpTxnClientPriorityGet(TSHttpTxn txnp,
> uint64_t *stream_dependency, int *weight);
> 
> I doubt removing "Http2" from this API makes sense. Because this API is
> assuming HTTP/2's priority scheme (dependency & weight).
> As Masakazu pointed out, a new priority scheme is proposed and it's
> completely different from HTTP/2's priority scheme.
> It's trying to make a priority scheme HTTP version-independent.

Rather than making the API protocol specific, why not use opaque structures, and a protocol “type” identifier returned? Even with H2 and later H3, it’s possible things like priority keeps changing.

The caller would of course have to type cast to the appropriate priority struct. However,  I don’t feel super strongly here, and sadly we decided not to turn the public APIs into C++ where this type of behavior is more natural.

— Leif
> 
> So I'd like to leave "Http2" for this API and reserve
> "TSHttpTxnClientPriorityGet" for the new priority scheme.
> 
> - Masaori
> 
>> On Tue, Sep 1, 2020 at 12:02 PM Brian Neradt <br...@gmail.com> wrote:
>> 
>> Thanks all, this is good feedback. It sounds like we're in agreement on
>> dropping the explicit reference to Http2 so this can accommodate future
>> protocols. Considering this and the observation that stream_id is 62 bits
>> for HTTP/3, the output parameters should be increased in size accordingly.
>> That would make the API look like the following:
>> 
>> tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp, uint64_t
>> *stream_id);
>> 
>> tsapi TSReturnCode TSHttpTxnClientPriorityGet(TSHttpTxn txnp, uint64_t
>> *stream_dependency, int *weight);
>> 
>> This will satisfy the Traffic Dump plugin needs. The concern raised about
>> protocol identification is not an issue for Traffic Dump because it will
>> know the protocol from either the protocol retrieval API
>> (TSHttpSsnClientProtocolStackGet) or from the version in the request
>> via TSHttpHdrVersionGet. That being the case, I'm content not adding a
>> separate output parameter for this.
>> 
>> That said, I don't want to dismiss the option too quickly if others are
>> still interested. Sudheer, you seem to have the most developed ideas for
>> this. If, considering what I say above, you'd still like us to consider the
>> inclusion of a context parameter, could you please flesh out:
>> 
>>   - What the signatures for the stream id and priority getters would look
>>   like.
>>   - The structure of the context object.
>>   - Would all HTTP/2 and above APIs contain this context parameter?
>> 
>> Otherwise, if we're OK not having the context parameter, I can update the
>> draft PR with the above changes to the API.
>> 
>> Thanks,
>> 
>> On Mon, Aug 31, 2020 at 9:19 PM Sudheer Vinukonda
>> <su...@yahoo.com.invalid> wrote:
>> 
>>> Right, it does make sense to have a generic API naming (and remove http2
>>> reference from it), but I'm thinking it should still allow to pass in
>> some
>>> context object (we may define an interface for it, with "protocol" being
>> a
>>> mandatory field) to be able to continue using it in the future as well as
>>> to let user explicitly specify that they want stream id for http2 or
>> http2
>>> for example?
>>> Thanks,
>>> Sudheer
>>>    On Monday, August 31, 2020, 06:49:57 PM PDT, Masakazu Kitajo <
>>> m4sk17@gmail.com> wrote:
>>> 
>>> Sudheer,
>>> 
>>> Would a separated API to get the actual protocol active for the given
>>> request satisfy you? I don't understand how indicating the actual
>> protocol
>>> helps something.
>>> 
>>> I think I understand your concern. Future protocols may need different
>>> params, or might have different representations. However, H/2 is
>> extensible
>>> so any APIs that have "Http2" in their names still can be incompatible if
>>> we support extensions. The proposal for a new priority scheme is a good
>>> example.
>>> 
>>> In that sense, passing a context object may be helpful, and more flexible
>>> than having fixed context in API names, but maybe it's too early to think
>>> about it.
>>> 
>>> Masakazu
>>> 
>>> 
>>> On Mon, Aug 31, 2020 at 11:05 AM Sudheer Vinukonda
>>> <su...@yahoo.com.invalid> wrote:
>>> 
>>>> Hmm, it does sound appealing to avoid having to duplicate API for
>> common
>>>> attributes between H/2 and H/3, but, I'm thinking the API should still
>>>> somehow explicitly indicate (either via an input param or an output
>>> param)
>>>> the actual protocol active for the given request. It seems a bit too
>>>> confusing (and potentially problematic depending on how things evolve
>> in
>>>> the future) to hide that aspect.
>>>> Thoughts?
>>>> 
>>>> 
>>>>   On Sunday, August 30, 2020, 06:56:49 PM PDT, Masakazu Kitajo <
>>>> maskit@apache.org> wrote:
>>>> 
>>>> And there is a proposal for a new priority scheme that may be used for
>>>> both
>>>> H2 and H3.
>>>> https://tools.ietf.org/html/draft-ietf-httpbis-priority-01
>>>> 
>>>> I haven't read it, but it would be nice if the TS API could be
>> compatible
>>>> with the new scheme.
>>>> 
>>>> On Mon, Aug 31, 2020 at 10:48 AM Masakazu Kitajo <ma...@apache.org>
>>>> wrote:
>>>> 
>>>>> I think we should avoid adding APIs just for HTTP/2 unless it's
>> really
>>>>> HTTP/2 specific. Are we going to add TSHttpTxnClientHttp3StreamIdGet
>>> and
>>>>> TSHttpTxnClientHttp3PriorityGet as well?
>>>>> 
>>>>> I'd omit the "2" in the API names, and rephrase the API doc like:
>>>>> This API returns an error if the provided transaction does not have a
>>>>> stream id.
>>>>> 
>>>>> Also, a stream id on HTTP/3 (QUIC) is 62 bit integer.
>>>>> 
>>>>> Masakazu
>>>>> 
>>>>> 
>>>>> On Sat, Aug 29, 2020 at 5:24 AM SUSAN HINRICHS <sh...@ieee.org>
>>>> wrote:
>>>>> 
>>>>>> +1
>>>>>> 
>>>>>> On Fri, Aug 28, 2020 at 3:23 PM Sudheer Vinukonda
>>>>>> <su...@yahoo.com.invalid> wrote:
>>>>>> 
>>>>>>> +1.
>>>>>>> Sounds reasonable to me.
>>>>>>> 
>>>>>>> 
>>>>>>>   On Friday, August 28, 2020, 07:41:18 AM PDT, Brian Neradt <
>>>>>>> brian.neradt@gmail.com> wrote:
>>>>>>> 
>>>>>>> Hi dev@trafficserver.apache.org,
>>>>>>> 
>>>>>>> It would be helpful to add HTTP/2 stream and priority support to
>>>> Traffic
>>>>>>> Dump. In order for the plugin to access this information, I
>> propose
>>>>>> adding
>>>>>>> the following two functions to the TS API:
>>>>>>> 
>>>>>>> tsapi TSReturnCode TSHttpTxnClientHttp2StreamIdGet(TSHttpTxn txnp,
>>>>>> uint32_t
>>>>>>> *stream_id);
>>>>>>> 
>>>>>>> tsapi TSReturnCode TSHttpTxnClientHttp2PriorityGet(TSHttpTxn txnp,
>>> int
>>>>>>> *stream_dependency, int *weight);
>>>>>>> 
>>>>>>> I've created a draft PR with this implemented along with Traffic
>>>> Dump's
>>>>>> use
>>>>>>> of it:
>>>>>>> https://github.com/apache/trafficserver/pull/7149
>>>>>>> 
>>>>>>> Please let me know if there are any concerns or suggestions for
>>>>>> improvement
>>>>>>> concerning this.
>>>>>>> 
>>>>>>> Thanks!
>>>>>>> Brian
>>>>>>> 
>>>>>>> --
>>>>>>> "Come to Me, all who are weary and heavy-laden, and I will
>>>>>>> give you rest. Take My yoke upon you and learn from Me, for
>>>>>>> I am gentle and humble in heart, and you will find rest for
>>>>>>> your souls. For My yoke is easy and My burden is light."
>>>>>>> 
>>>>>>>   ~ Matthew 11:28-30
>>>>>>> 
>>>>>> 
>>>>> 
>>>> 
>>> 
>> 
>> 
>> 
>> --
>> "Come to Me, all who are weary and heavy-laden, and I will
>> give you rest. Take My yoke upon you and learn from Me, for
>> I am gentle and humble in heart, and you will find rest for
>> your souls. For My yoke is easy and My burden is light."
>> 
>>    ~ Matthew 11:28-30
>> 


Re: TS API Review: New HTTP/2 stream id and priority getters

Posted by Masaori Koshiba <ma...@apache.org>.
I totally agree with less API is better.

> tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp,
uint64_t *stream_id);

Stripping "Http2" looks reasonable. We can use this API for protocols that
have stream id.

> tsapi TSReturnCode TSHttpTxnClientPriorityGet(TSHttpTxn txnp,
uint64_t *stream_dependency, int *weight);

I doubt removing "Http2" from this API makes sense. Because this API is
assuming HTTP/2's priority scheme (dependency & weight).
As Masakazu pointed out, a new priority scheme is proposed and it's
completely different from HTTP/2's priority scheme.
It's trying to make a priority scheme HTTP version-independent.

So I'd like to leave "Http2" for this API and reserve
"TSHttpTxnClientPriorityGet" for the new priority scheme.

- Masaori

On Tue, Sep 1, 2020 at 12:02 PM Brian Neradt <br...@gmail.com> wrote:

> Thanks all, this is good feedback. It sounds like we're in agreement on
> dropping the explicit reference to Http2 so this can accommodate future
> protocols. Considering this and the observation that stream_id is 62 bits
> for HTTP/3, the output parameters should be increased in size accordingly.
> That would make the API look like the following:
>
> tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp, uint64_t
> *stream_id);
>
> tsapi TSReturnCode TSHttpTxnClientPriorityGet(TSHttpTxn txnp, uint64_t
> *stream_dependency, int *weight);
>
> This will satisfy the Traffic Dump plugin needs. The concern raised about
> protocol identification is not an issue for Traffic Dump because it will
> know the protocol from either the protocol retrieval API
> (TSHttpSsnClientProtocolStackGet) or from the version in the request
> via TSHttpHdrVersionGet. That being the case, I'm content not adding a
> separate output parameter for this.
>
> That said, I don't want to dismiss the option too quickly if others are
> still interested. Sudheer, you seem to have the most developed ideas for
> this. If, considering what I say above, you'd still like us to consider the
> inclusion of a context parameter, could you please flesh out:
>
>    - What the signatures for the stream id and priority getters would look
>    like.
>    - The structure of the context object.
>    - Would all HTTP/2 and above APIs contain this context parameter?
>
> Otherwise, if we're OK not having the context parameter, I can update the
> draft PR with the above changes to the API.
>
> Thanks,
>
> On Mon, Aug 31, 2020 at 9:19 PM Sudheer Vinukonda
> <su...@yahoo.com.invalid> wrote:
>
> >  Right, it does make sense to have a generic API naming (and remove http2
> > reference from it), but I'm thinking it should still allow to pass in
> some
> > context object (we may define an interface for it, with "protocol" being
> a
> > mandatory field) to be able to continue using it in the future as well as
> > to let user explicitly specify that they want stream id for http2 or
> http2
> > for example?
> > Thanks,
> > Sudheer
> >     On Monday, August 31, 2020, 06:49:57 PM PDT, Masakazu Kitajo <
> > m4sk17@gmail.com> wrote:
> >
> >  Sudheer,
> >
> > Would a separated API to get the actual protocol active for the given
> > request satisfy you? I don't understand how indicating the actual
> protocol
> > helps something.
> >
> > I think I understand your concern. Future protocols may need different
> > params, or might have different representations. However, H/2 is
> extensible
> > so any APIs that have "Http2" in their names still can be incompatible if
> > we support extensions. The proposal for a new priority scheme is a good
> > example.
> >
> > In that sense, passing a context object may be helpful, and more flexible
> > than having fixed context in API names, but maybe it's too early to think
> > about it.
> >
> > Masakazu
> >
> >
> > On Mon, Aug 31, 2020 at 11:05 AM Sudheer Vinukonda
> > <su...@yahoo.com.invalid> wrote:
> >
> > >  Hmm, it does sound appealing to avoid having to duplicate API for
> common
> > > attributes between H/2 and H/3, but, I'm thinking the API should still
> > > somehow explicitly indicate (either via an input param or an output
> > param)
> > > the actual protocol active for the given request. It seems a bit too
> > > confusing (and potentially problematic depending on how things evolve
> in
> > > the future) to hide that aspect.
> > > Thoughts?
> > >
> > >
> > >    On Sunday, August 30, 2020, 06:56:49 PM PDT, Masakazu Kitajo <
> > > maskit@apache.org> wrote:
> > >
> > >  And there is a proposal for a new priority scheme that may be used for
> > > both
> > > H2 and H3.
> > > https://tools.ietf.org/html/draft-ietf-httpbis-priority-01
> > >
> > > I haven't read it, but it would be nice if the TS API could be
> compatible
> > > with the new scheme.
> > >
> > > On Mon, Aug 31, 2020 at 10:48 AM Masakazu Kitajo <ma...@apache.org>
> > > wrote:
> > >
> > > > I think we should avoid adding APIs just for HTTP/2 unless it's
> really
> > > > HTTP/2 specific. Are we going to add TSHttpTxnClientHttp3StreamIdGet
> > and
> > > > TSHttpTxnClientHttp3PriorityGet as well?
> > > >
> > > > I'd omit the "2" in the API names, and rephrase the API doc like:
> > > > This API returns an error if the provided transaction does not have a
> > > > stream id.
> > > >
> > > > Also, a stream id on HTTP/3 (QUIC) is 62 bit integer.
> > > >
> > > > Masakazu
> > > >
> > > >
> > > > On Sat, Aug 29, 2020 at 5:24 AM SUSAN HINRICHS <sh...@ieee.org>
> > > wrote:
> > > >
> > > >> +1
> > > >>
> > > >> On Fri, Aug 28, 2020 at 3:23 PM Sudheer Vinukonda
> > > >> <su...@yahoo.com.invalid> wrote:
> > > >>
> > > >> >  +1.
> > > >> > Sounds reasonable to me.
> > > >> >
> > > >> >
> > > >> >    On Friday, August 28, 2020, 07:41:18 AM PDT, Brian Neradt <
> > > >> > brian.neradt@gmail.com> wrote:
> > > >> >
> > > >> >  Hi dev@trafficserver.apache.org,
> > > >> >
> > > >> > It would be helpful to add HTTP/2 stream and priority support to
> > > Traffic
> > > >> > Dump. In order for the plugin to access this information, I
> propose
> > > >> adding
> > > >> > the following two functions to the TS API:
> > > >> >
> > > >> > tsapi TSReturnCode TSHttpTxnClientHttp2StreamIdGet(TSHttpTxn txnp,
> > > >> uint32_t
> > > >> > *stream_id);
> > > >> >
> > > >> > tsapi TSReturnCode TSHttpTxnClientHttp2PriorityGet(TSHttpTxn txnp,
> > int
> > > >> > *stream_dependency, int *weight);
> > > >> >
> > > >> > I've created a draft PR with this implemented along with Traffic
> > > Dump's
> > > >> use
> > > >> > of it:
> > > >> > https://github.com/apache/trafficserver/pull/7149
> > > >> >
> > > >> > Please let me know if there are any concerns or suggestions for
> > > >> improvement
> > > >> > concerning this.
> > > >> >
> > > >> > Thanks!
> > > >> > Brian
> > > >> >
> > > >> > --
> > > >> > "Come to Me, all who are weary and heavy-laden, and I will
> > > >> > give you rest. Take My yoke upon you and learn from Me, for
> > > >> > I am gentle and humble in heart, and you will find rest for
> > > >> > your souls. For My yoke is easy and My burden is light."
> > > >> >
> > > >> >    ~ Matthew 11:28-30
> > > >> >
> > > >>
> > > >
> > >
> >
>
>
>
> --
> "Come to Me, all who are weary and heavy-laden, and I will
> give you rest. Take My yoke upon you and learn from Me, for
> I am gentle and humble in heart, and you will find rest for
> your souls. For My yoke is easy and My burden is light."
>
>     ~ Matthew 11:28-30
>

Re: TS API Review: New HTTP/2 stream id and priority getters

Posted by Brian Neradt <br...@gmail.com>.
Thanks all, this is good feedback. It sounds like we're in agreement on
dropping the explicit reference to Http2 so this can accommodate future
protocols. Considering this and the observation that stream_id is 62 bits
for HTTP/3, the output parameters should be increased in size accordingly.
That would make the API look like the following:

tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp, uint64_t
*stream_id);

tsapi TSReturnCode TSHttpTxnClientPriorityGet(TSHttpTxn txnp, uint64_t
*stream_dependency, int *weight);

This will satisfy the Traffic Dump plugin needs. The concern raised about
protocol identification is not an issue for Traffic Dump because it will
know the protocol from either the protocol retrieval API
(TSHttpSsnClientProtocolStackGet) or from the version in the request
via TSHttpHdrVersionGet. That being the case, I'm content not adding a
separate output parameter for this.

That said, I don't want to dismiss the option too quickly if others are
still interested. Sudheer, you seem to have the most developed ideas for
this. If, considering what I say above, you'd still like us to consider the
inclusion of a context parameter, could you please flesh out:

   - What the signatures for the stream id and priority getters would look
   like.
   - The structure of the context object.
   - Would all HTTP/2 and above APIs contain this context parameter?

Otherwise, if we're OK not having the context parameter, I can update the
draft PR with the above changes to the API.

Thanks,

On Mon, Aug 31, 2020 at 9:19 PM Sudheer Vinukonda
<su...@yahoo.com.invalid> wrote:

>  Right, it does make sense to have a generic API naming (and remove http2
> reference from it), but I'm thinking it should still allow to pass in some
> context object (we may define an interface for it, with "protocol" being a
> mandatory field) to be able to continue using it in the future as well as
> to let user explicitly specify that they want stream id for http2 or http2
> for example?
> Thanks,
> Sudheer
>     On Monday, August 31, 2020, 06:49:57 PM PDT, Masakazu Kitajo <
> m4sk17@gmail.com> wrote:
>
>  Sudheer,
>
> Would a separated API to get the actual protocol active for the given
> request satisfy you? I don't understand how indicating the actual protocol
> helps something.
>
> I think I understand your concern. Future protocols may need different
> params, or might have different representations. However, H/2 is extensible
> so any APIs that have "Http2" in their names still can be incompatible if
> we support extensions. The proposal for a new priority scheme is a good
> example.
>
> In that sense, passing a context object may be helpful, and more flexible
> than having fixed context in API names, but maybe it's too early to think
> about it.
>
> Masakazu
>
>
> On Mon, Aug 31, 2020 at 11:05 AM Sudheer Vinukonda
> <su...@yahoo.com.invalid> wrote:
>
> >  Hmm, it does sound appealing to avoid having to duplicate API for common
> > attributes between H/2 and H/3, but, I'm thinking the API should still
> > somehow explicitly indicate (either via an input param or an output
> param)
> > the actual protocol active for the given request. It seems a bit too
> > confusing (and potentially problematic depending on how things evolve in
> > the future) to hide that aspect.
> > Thoughts?
> >
> >
> >    On Sunday, August 30, 2020, 06:56:49 PM PDT, Masakazu Kitajo <
> > maskit@apache.org> wrote:
> >
> >  And there is a proposal for a new priority scheme that may be used for
> > both
> > H2 and H3.
> > https://tools.ietf.org/html/draft-ietf-httpbis-priority-01
> >
> > I haven't read it, but it would be nice if the TS API could be compatible
> > with the new scheme.
> >
> > On Mon, Aug 31, 2020 at 10:48 AM Masakazu Kitajo <ma...@apache.org>
> > wrote:
> >
> > > I think we should avoid adding APIs just for HTTP/2 unless it's really
> > > HTTP/2 specific. Are we going to add TSHttpTxnClientHttp3StreamIdGet
> and
> > > TSHttpTxnClientHttp3PriorityGet as well?
> > >
> > > I'd omit the "2" in the API names, and rephrase the API doc like:
> > > This API returns an error if the provided transaction does not have a
> > > stream id.
> > >
> > > Also, a stream id on HTTP/3 (QUIC) is 62 bit integer.
> > >
> > > Masakazu
> > >
> > >
> > > On Sat, Aug 29, 2020 at 5:24 AM SUSAN HINRICHS <sh...@ieee.org>
> > wrote:
> > >
> > >> +1
> > >>
> > >> On Fri, Aug 28, 2020 at 3:23 PM Sudheer Vinukonda
> > >> <su...@yahoo.com.invalid> wrote:
> > >>
> > >> >  +1.
> > >> > Sounds reasonable to me.
> > >> >
> > >> >
> > >> >    On Friday, August 28, 2020, 07:41:18 AM PDT, Brian Neradt <
> > >> > brian.neradt@gmail.com> wrote:
> > >> >
> > >> >  Hi dev@trafficserver.apache.org,
> > >> >
> > >> > It would be helpful to add HTTP/2 stream and priority support to
> > Traffic
> > >> > Dump. In order for the plugin to access this information, I propose
> > >> adding
> > >> > the following two functions to the TS API:
> > >> >
> > >> > tsapi TSReturnCode TSHttpTxnClientHttp2StreamIdGet(TSHttpTxn txnp,
> > >> uint32_t
> > >> > *stream_id);
> > >> >
> > >> > tsapi TSReturnCode TSHttpTxnClientHttp2PriorityGet(TSHttpTxn txnp,
> int
> > >> > *stream_dependency, int *weight);
> > >> >
> > >> > I've created a draft PR with this implemented along with Traffic
> > Dump's
> > >> use
> > >> > of it:
> > >> > https://github.com/apache/trafficserver/pull/7149
> > >> >
> > >> > Please let me know if there are any concerns or suggestions for
> > >> improvement
> > >> > concerning this.
> > >> >
> > >> > Thanks!
> > >> > Brian
> > >> >
> > >> > --
> > >> > "Come to Me, all who are weary and heavy-laden, and I will
> > >> > give you rest. Take My yoke upon you and learn from Me, for
> > >> > I am gentle and humble in heart, and you will find rest for
> > >> > your souls. For My yoke is easy and My burden is light."
> > >> >
> > >> >    ~ Matthew 11:28-30
> > >> >
> > >>
> > >
> >
>



-- 
"Come to Me, all who are weary and heavy-laden, and I will
give you rest. Take My yoke upon you and learn from Me, for
I am gentle and humble in heart, and you will find rest for
your souls. For My yoke is easy and My burden is light."

    ~ Matthew 11:28-30

Re: TS API Review: New HTTP/2 stream id and priority getters

Posted by Sudheer Vinukonda <su...@yahoo.com.INVALID>.
 Right, it does make sense to have a generic API naming (and remove http2 reference from it), but I'm thinking it should still allow to pass in some context object (we may define an interface for it, with "protocol" being a mandatory field) to be able to continue using it in the future as well as to let user explicitly specify that they want stream id for http2 or http2 for example?
Thanks,
Sudheer
    On Monday, August 31, 2020, 06:49:57 PM PDT, Masakazu Kitajo <m4...@gmail.com> wrote:  
 
 Sudheer,

Would a separated API to get the actual protocol active for the given
request satisfy you? I don't understand how indicating the actual protocol
helps something.

I think I understand your concern. Future protocols may need different
params, or might have different representations. However, H/2 is extensible
so any APIs that have "Http2" in their names still can be incompatible if
we support extensions. The proposal for a new priority scheme is a good
example.

In that sense, passing a context object may be helpful, and more flexible
than having fixed context in API names, but maybe it's too early to think
about it.

Masakazu


On Mon, Aug 31, 2020 at 11:05 AM Sudheer Vinukonda
<su...@yahoo.com.invalid> wrote:

>  Hmm, it does sound appealing to avoid having to duplicate API for common
> attributes between H/2 and H/3, but, I'm thinking the API should still
> somehow explicitly indicate (either via an input param or an output param)
> the actual protocol active for the given request. It seems a bit too
> confusing (and potentially problematic depending on how things evolve in
> the future) to hide that aspect.
> Thoughts?
>
>
>    On Sunday, August 30, 2020, 06:56:49 PM PDT, Masakazu Kitajo <
> maskit@apache.org> wrote:
>
>  And there is a proposal for a new priority scheme that may be used for
> both
> H2 and H3.
> https://tools.ietf.org/html/draft-ietf-httpbis-priority-01
>
> I haven't read it, but it would be nice if the TS API could be compatible
> with the new scheme.
>
> On Mon, Aug 31, 2020 at 10:48 AM Masakazu Kitajo <ma...@apache.org>
> wrote:
>
> > I think we should avoid adding APIs just for HTTP/2 unless it's really
> > HTTP/2 specific. Are we going to add TSHttpTxnClientHttp3StreamIdGet and
> > TSHttpTxnClientHttp3PriorityGet as well?
> >
> > I'd omit the "2" in the API names, and rephrase the API doc like:
> > This API returns an error if the provided transaction does not have a
> > stream id.
> >
> > Also, a stream id on HTTP/3 (QUIC) is 62 bit integer.
> >
> > Masakazu
> >
> >
> > On Sat, Aug 29, 2020 at 5:24 AM SUSAN HINRICHS <sh...@ieee.org>
> wrote:
> >
> >> +1
> >>
> >> On Fri, Aug 28, 2020 at 3:23 PM Sudheer Vinukonda
> >> <su...@yahoo.com.invalid> wrote:
> >>
> >> >  +1.
> >> > Sounds reasonable to me.
> >> >
> >> >
> >> >    On Friday, August 28, 2020, 07:41:18 AM PDT, Brian Neradt <
> >> > brian.neradt@gmail.com> wrote:
> >> >
> >> >  Hi dev@trafficserver.apache.org,
> >> >
> >> > It would be helpful to add HTTP/2 stream and priority support to
> Traffic
> >> > Dump. In order for the plugin to access this information, I propose
> >> adding
> >> > the following two functions to the TS API:
> >> >
> >> > tsapi TSReturnCode TSHttpTxnClientHttp2StreamIdGet(TSHttpTxn txnp,
> >> uint32_t
> >> > *stream_id);
> >> >
> >> > tsapi TSReturnCode TSHttpTxnClientHttp2PriorityGet(TSHttpTxn txnp, int
> >> > *stream_dependency, int *weight);
> >> >
> >> > I've created a draft PR with this implemented along with Traffic
> Dump's
> >> use
> >> > of it:
> >> > https://github.com/apache/trafficserver/pull/7149
> >> >
> >> > Please let me know if there are any concerns or suggestions for
> >> improvement
> >> > concerning this.
> >> >
> >> > Thanks!
> >> > Brian
> >> >
> >> > --
> >> > "Come to Me, all who are weary and heavy-laden, and I will
> >> > give you rest. Take My yoke upon you and learn from Me, for
> >> > I am gentle and humble in heart, and you will find rest for
> >> > your souls. For My yoke is easy and My burden is light."
> >> >
> >> >    ~ Matthew 11:28-30
> >> >
> >>
> >
>
  

Re: TS API Review: New HTTP/2 stream id and priority getters

Posted by Masakazu Kitajo <m4...@gmail.com>.
Sudheer,

Would a separated API to get the actual protocol active for the given
request satisfy you? I don't understand how indicating the actual protocol
helps something.

I think I understand your concern. Future protocols may need different
params, or might have different representations. However, H/2 is extensible
so any APIs that have "Http2" in their names still can be incompatible if
we support extensions. The proposal for a new priority scheme is a good
example.

In that sense, passing a context object may be helpful, and more flexible
than having fixed context in API names, but maybe it's too early to think
about it.

Masakazu


On Mon, Aug 31, 2020 at 11:05 AM Sudheer Vinukonda
<su...@yahoo.com.invalid> wrote:

>  Hmm, it does sound appealing to avoid having to duplicate API for common
> attributes between H/2 and H/3, but, I'm thinking the API should still
> somehow explicitly indicate (either via an input param or an output param)
> the actual protocol active for the given request. It seems a bit too
> confusing (and potentially problematic depending on how things evolve in
> the future) to hide that aspect.
> Thoughts?
>
>
>     On Sunday, August 30, 2020, 06:56:49 PM PDT, Masakazu Kitajo <
> maskit@apache.org> wrote:
>
>  And there is a proposal for a new priority scheme that may be used for
> both
> H2 and H3.
> https://tools.ietf.org/html/draft-ietf-httpbis-priority-01
>
> I haven't read it, but it would be nice if the TS API could be compatible
> with the new scheme.
>
> On Mon, Aug 31, 2020 at 10:48 AM Masakazu Kitajo <ma...@apache.org>
> wrote:
>
> > I think we should avoid adding APIs just for HTTP/2 unless it's really
> > HTTP/2 specific. Are we going to add TSHttpTxnClientHttp3StreamIdGet and
> > TSHttpTxnClientHttp3PriorityGet as well?
> >
> > I'd omit the "2" in the API names, and rephrase the API doc like:
> > This API returns an error if the provided transaction does not have a
> > stream id.
> >
> > Also, a stream id on HTTP/3 (QUIC) is 62 bit integer.
> >
> > Masakazu
> >
> >
> > On Sat, Aug 29, 2020 at 5:24 AM SUSAN HINRICHS <sh...@ieee.org>
> wrote:
> >
> >> +1
> >>
> >> On Fri, Aug 28, 2020 at 3:23 PM Sudheer Vinukonda
> >> <su...@yahoo.com.invalid> wrote:
> >>
> >> >  +1.
> >> > Sounds reasonable to me.
> >> >
> >> >
> >> >    On Friday, August 28, 2020, 07:41:18 AM PDT, Brian Neradt <
> >> > brian.neradt@gmail.com> wrote:
> >> >
> >> >  Hi dev@trafficserver.apache.org,
> >> >
> >> > It would be helpful to add HTTP/2 stream and priority support to
> Traffic
> >> > Dump. In order for the plugin to access this information, I propose
> >> adding
> >> > the following two functions to the TS API:
> >> >
> >> > tsapi TSReturnCode TSHttpTxnClientHttp2StreamIdGet(TSHttpTxn txnp,
> >> uint32_t
> >> > *stream_id);
> >> >
> >> > tsapi TSReturnCode TSHttpTxnClientHttp2PriorityGet(TSHttpTxn txnp, int
> >> > *stream_dependency, int *weight);
> >> >
> >> > I've created a draft PR with this implemented along with Traffic
> Dump's
> >> use
> >> > of it:
> >> > https://github.com/apache/trafficserver/pull/7149
> >> >
> >> > Please let me know if there are any concerns or suggestions for
> >> improvement
> >> > concerning this.
> >> >
> >> > Thanks!
> >> > Brian
> >> >
> >> > --
> >> > "Come to Me, all who are weary and heavy-laden, and I will
> >> > give you rest. Take My yoke upon you and learn from Me, for
> >> > I am gentle and humble in heart, and you will find rest for
> >> > your souls. For My yoke is easy and My burden is light."
> >> >
> >> >    ~ Matthew 11:28-30
> >> >
> >>
> >
>

Re: TS API Review: New HTTP/2 stream id and priority getters

Posted by Sudheer Vinukonda <su...@yahoo.com.INVALID>.
 Hmm, it does sound appealing to avoid having to duplicate API for common attributes between H/2 and H/3, but, I'm thinking the API should still somehow explicitly indicate (either via an input param or an output param) the actual protocol active for the given request. It seems a bit too confusing (and potentially problematic depending on how things evolve in the future) to hide that aspect.
Thoughts?


    On Sunday, August 30, 2020, 06:56:49 PM PDT, Masakazu Kitajo <ma...@apache.org> wrote:  
 
 And there is a proposal for a new priority scheme that may be used for both
H2 and H3.
https://tools.ietf.org/html/draft-ietf-httpbis-priority-01

I haven't read it, but it would be nice if the TS API could be compatible
with the new scheme.

On Mon, Aug 31, 2020 at 10:48 AM Masakazu Kitajo <ma...@apache.org> wrote:

> I think we should avoid adding APIs just for HTTP/2 unless it's really
> HTTP/2 specific. Are we going to add TSHttpTxnClientHttp3StreamIdGet and
> TSHttpTxnClientHttp3PriorityGet as well?
>
> I'd omit the "2" in the API names, and rephrase the API doc like:
> This API returns an error if the provided transaction does not have a
> stream id.
>
> Also, a stream id on HTTP/3 (QUIC) is 62 bit integer.
>
> Masakazu
>
>
> On Sat, Aug 29, 2020 at 5:24 AM SUSAN HINRICHS <sh...@ieee.org> wrote:
>
>> +1
>>
>> On Fri, Aug 28, 2020 at 3:23 PM Sudheer Vinukonda
>> <su...@yahoo.com.invalid> wrote:
>>
>> >  +1.
>> > Sounds reasonable to me.
>> >
>> >
>> >    On Friday, August 28, 2020, 07:41:18 AM PDT, Brian Neradt <
>> > brian.neradt@gmail.com> wrote:
>> >
>> >  Hi dev@trafficserver.apache.org,
>> >
>> > It would be helpful to add HTTP/2 stream and priority support to Traffic
>> > Dump. In order for the plugin to access this information, I propose
>> adding
>> > the following two functions to the TS API:
>> >
>> > tsapi TSReturnCode TSHttpTxnClientHttp2StreamIdGet(TSHttpTxn txnp,
>> uint32_t
>> > *stream_id);
>> >
>> > tsapi TSReturnCode TSHttpTxnClientHttp2PriorityGet(TSHttpTxn txnp, int
>> > *stream_dependency, int *weight);
>> >
>> > I've created a draft PR with this implemented along with Traffic Dump's
>> use
>> > of it:
>> > https://github.com/apache/trafficserver/pull/7149
>> >
>> > Please let me know if there are any concerns or suggestions for
>> improvement
>> > concerning this.
>> >
>> > Thanks!
>> > Brian
>> >
>> > --
>> > "Come to Me, all who are weary and heavy-laden, and I will
>> > give you rest. Take My yoke upon you and learn from Me, for
>> > I am gentle and humble in heart, and you will find rest for
>> > your souls. For My yoke is easy and My burden is light."
>> >
>> >    ~ Matthew 11:28-30
>> >
>>
>
  

Re: TS API Review: New HTTP/2 stream id and priority getters

Posted by Masakazu Kitajo <ma...@apache.org>.
And there is a proposal for a new priority scheme that may be used for both
H2 and H3.
https://tools.ietf.org/html/draft-ietf-httpbis-priority-01

I haven't read it, but it would be nice if the TS API could be compatible
with the new scheme.

On Mon, Aug 31, 2020 at 10:48 AM Masakazu Kitajo <ma...@apache.org> wrote:

> I think we should avoid adding APIs just for HTTP/2 unless it's really
> HTTP/2 specific. Are we going to add TSHttpTxnClientHttp3StreamIdGet and
> TSHttpTxnClientHttp3PriorityGet as well?
>
> I'd omit the "2" in the API names, and rephrase the API doc like:
> This API returns an error if the provided transaction does not have a
> stream id.
>
> Also, a stream id on HTTP/3 (QUIC) is 62 bit integer.
>
> Masakazu
>
>
> On Sat, Aug 29, 2020 at 5:24 AM SUSAN HINRICHS <sh...@ieee.org> wrote:
>
>> +1
>>
>> On Fri, Aug 28, 2020 at 3:23 PM Sudheer Vinukonda
>> <su...@yahoo.com.invalid> wrote:
>>
>> >  +1.
>> > Sounds reasonable to me.
>> >
>> >
>> >     On Friday, August 28, 2020, 07:41:18 AM PDT, Brian Neradt <
>> > brian.neradt@gmail.com> wrote:
>> >
>> >  Hi dev@trafficserver.apache.org,
>> >
>> > It would be helpful to add HTTP/2 stream and priority support to Traffic
>> > Dump. In order for the plugin to access this information, I propose
>> adding
>> > the following two functions to the TS API:
>> >
>> > tsapi TSReturnCode TSHttpTxnClientHttp2StreamIdGet(TSHttpTxn txnp,
>> uint32_t
>> > *stream_id);
>> >
>> > tsapi TSReturnCode TSHttpTxnClientHttp2PriorityGet(TSHttpTxn txnp, int
>> > *stream_dependency, int *weight);
>> >
>> > I've created a draft PR with this implemented along with Traffic Dump's
>> use
>> > of it:
>> > https://github.com/apache/trafficserver/pull/7149
>> >
>> > Please let me know if there are any concerns or suggestions for
>> improvement
>> > concerning this.
>> >
>> > Thanks!
>> > Brian
>> >
>> > --
>> > "Come to Me, all who are weary and heavy-laden, and I will
>> > give you rest. Take My yoke upon you and learn from Me, for
>> > I am gentle and humble in heart, and you will find rest for
>> > your souls. For My yoke is easy and My burden is light."
>> >
>> >     ~ Matthew 11:28-30
>> >
>>
>

Re: TS API Review: New HTTP/2 stream id and priority getters

Posted by Masakazu Kitajo <ma...@apache.org>.
I think we should avoid adding APIs just for HTTP/2 unless it's really
HTTP/2 specific. Are we going to add TSHttpTxnClientHttp3StreamIdGet and
TSHttpTxnClientHttp3PriorityGet as well?

I'd omit the "2" in the API names, and rephrase the API doc like:
This API returns an error if the provided transaction does not have a
stream id.

Also, a stream id on HTTP/3 (QUIC) is 62 bit integer.

Masakazu


On Sat, Aug 29, 2020 at 5:24 AM SUSAN HINRICHS <sh...@ieee.org> wrote:

> +1
>
> On Fri, Aug 28, 2020 at 3:23 PM Sudheer Vinukonda
> <su...@yahoo.com.invalid> wrote:
>
> >  +1.
> > Sounds reasonable to me.
> >
> >
> >     On Friday, August 28, 2020, 07:41:18 AM PDT, Brian Neradt <
> > brian.neradt@gmail.com> wrote:
> >
> >  Hi dev@trafficserver.apache.org,
> >
> > It would be helpful to add HTTP/2 stream and priority support to Traffic
> > Dump. In order for the plugin to access this information, I propose
> adding
> > the following two functions to the TS API:
> >
> > tsapi TSReturnCode TSHttpTxnClientHttp2StreamIdGet(TSHttpTxn txnp,
> uint32_t
> > *stream_id);
> >
> > tsapi TSReturnCode TSHttpTxnClientHttp2PriorityGet(TSHttpTxn txnp, int
> > *stream_dependency, int *weight);
> >
> > I've created a draft PR with this implemented along with Traffic Dump's
> use
> > of it:
> > https://github.com/apache/trafficserver/pull/7149
> >
> > Please let me know if there are any concerns or suggestions for
> improvement
> > concerning this.
> >
> > Thanks!
> > Brian
> >
> > --
> > "Come to Me, all who are weary and heavy-laden, and I will
> > give you rest. Take My yoke upon you and learn from Me, for
> > I am gentle and humble in heart, and you will find rest for
> > your souls. For My yoke is easy and My burden is light."
> >
> >     ~ Matthew 11:28-30
> >
>

Re: TS API Review: New HTTP/2 stream id and priority getters

Posted by SUSAN HINRICHS <sh...@ieee.org>.
+1

On Fri, Aug 28, 2020 at 3:23 PM Sudheer Vinukonda
<su...@yahoo.com.invalid> wrote:

>  +1.
> Sounds reasonable to me.
>
>
>     On Friday, August 28, 2020, 07:41:18 AM PDT, Brian Neradt <
> brian.neradt@gmail.com> wrote:
>
>  Hi dev@trafficserver.apache.org,
>
> It would be helpful to add HTTP/2 stream and priority support to Traffic
> Dump. In order for the plugin to access this information, I propose adding
> the following two functions to the TS API:
>
> tsapi TSReturnCode TSHttpTxnClientHttp2StreamIdGet(TSHttpTxn txnp, uint32_t
> *stream_id);
>
> tsapi TSReturnCode TSHttpTxnClientHttp2PriorityGet(TSHttpTxn txnp, int
> *stream_dependency, int *weight);
>
> I've created a draft PR with this implemented along with Traffic Dump's use
> of it:
> https://github.com/apache/trafficserver/pull/7149
>
> Please let me know if there are any concerns or suggestions for improvement
> concerning this.
>
> Thanks!
> Brian
>
> --
> "Come to Me, all who are weary and heavy-laden, and I will
> give you rest. Take My yoke upon you and learn from Me, for
> I am gentle and humble in heart, and you will find rest for
> your souls. For My yoke is easy and My burden is light."
>
>     ~ Matthew 11:28-30
>

Re: TS API Review: New HTTP/2 stream id and priority getters

Posted by Sudheer Vinukonda <su...@yahoo.com.INVALID>.
 +1.
Sounds reasonable to me.


    On Friday, August 28, 2020, 07:41:18 AM PDT, Brian Neradt <br...@gmail.com> wrote:  
 
 Hi dev@trafficserver.apache.org,

It would be helpful to add HTTP/2 stream and priority support to Traffic
Dump. In order for the plugin to access this information, I propose adding
the following two functions to the TS API:

tsapi TSReturnCode TSHttpTxnClientHttp2StreamIdGet(TSHttpTxn txnp, uint32_t
*stream_id);

tsapi TSReturnCode TSHttpTxnClientHttp2PriorityGet(TSHttpTxn txnp, int
*stream_dependency, int *weight);

I've created a draft PR with this implemented along with Traffic Dump's use
of it:
https://github.com/apache/trafficserver/pull/7149

Please let me know if there are any concerns or suggestions for improvement
concerning this.

Thanks!
Brian

-- 
"Come to Me, all who are weary and heavy-laden, and I will
give you rest. Take My yoke upon you and learn from Me, for
I am gentle and humble in heart, and you will find rest for
your souls. For My yoke is easy and My burden is light."

    ~ Matthew 11:28-30
  

Re: [E] TS API Review: New HTTP/2 stream id and priority getters

Posted by Walt Karas <wk...@verizonmedia.com.INVALID>.
Would traffic replay use this information?  More generally, what I'm
wondering is, how does traffic dump coordinate with TS's logging capability?

I think typically, the TS API provides these sorts of handles to plugins so
they can change the processing of the transaction/session/etc.  We could
also consider adding an API function that would return log fields in the
TXN_CLOSE hook.

On Fri, Aug 28, 2020 at 9:43 AM Brian Neradt <br...@gmail.com> wrote:

> Hi dev@trafficserver.apache.org,
>
> It would be helpful to add HTTP/2 stream and priority support to Traffic
> Dump. In order for the plugin to access this information, I propose adding
> the following two functions to the TS API:
>
> tsapi TSReturnCode TSHttpTxnClientHttp2StreamIdGet(TSHttpTxn txnp, uint32_t
> *stream_id);
>
> tsapi TSReturnCode TSHttpTxnClientHttp2PriorityGet(TSHttpTxn txnp, int
> *stream_dependency, int *weight);
>
> I've created a draft PR with this implemented along with Traffic Dump's use
> of it:
>
> https://urldefense.com/v3/__https://github.com/apache/trafficserver/pull/7149__;!!Op6eflyXZCqGR5I!VE1M67o_6YLQnYxkTvZVSCMAX1AnvdGwQ3lVwnMPiI2JXik1jP4KBcS2jg7KmrcJbg$
>
> Please let me know if there are any concerns or suggestions for improvement
> concerning this.
>
> Thanks!
> Brian
>
> --
> "Come to Me, all who are weary and heavy-laden, and I will
> give you rest. Take My yoke upon you and learn from Me, for
> I am gentle and humble in heart, and you will find rest for
> your souls. For My yoke is easy and My burden is light."
>
>     ~ Matthew 11:28-30
>

Re: TS API Review: New HTTP/2 stream id and priority getters

Posted by Bryan Call <bc...@apache.org>.
+1

-Bryan


> On Aug 28, 2020, at 7:40 AM, Brian Neradt <br...@gmail.com> wrote:
> 
> Hi dev@trafficserver.apache.org,
> 
> It would be helpful to add HTTP/2 stream and priority support to Traffic
> Dump. In order for the plugin to access this information, I propose adding
> the following two functions to the TS API:
> 
> tsapi TSReturnCode TSHttpTxnClientHttp2StreamIdGet(TSHttpTxn txnp, uint32_t
> *stream_id);
> 
> tsapi TSReturnCode TSHttpTxnClientHttp2PriorityGet(TSHttpTxn txnp, int
> *stream_dependency, int *weight);
> 
> I've created a draft PR with this implemented along with Traffic Dump's use
> of it:
> https://github.com/apache/trafficserver/pull/7149
> 
> Please let me know if there are any concerns or suggestions for improvement
> concerning this.
> 
> Thanks!
> Brian
> 
> -- 
> "Come to Me, all who are weary and heavy-laden, and I will
> give you rest. Take My yoke upon you and learn from Me, for
> I am gentle and humble in heart, and you will find rest for
> your souls. For My yoke is easy and My burden is light."
> 
>    ~ Matthew 11:28-30