>> (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
<sudheervinuko...@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 <masa...@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 <brian.ner...@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
>> <sudheervinuko...@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 <
>>> m4s...@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
>>> <sudheervinuko...@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 <
>>>> mas...@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 <mas...@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 <shinr...@ieee.org>
>>>> wrote:
>>>>>
>>>>>> +1
>>>>>>
>>>>>> On Fri, Aug 28, 2020 at 3:23 PM Sudheer Vinukonda
>>>>>> <sudheervinuko...@yahoo.com.invalid> wrote:
>>>>>>
>>>>>>> +1.
>>>>>>> Sounds reasonable to me.
>>>>>>>
>>>>>>>
>>>>>>> On Friday, August 28, 2020, 07:41:18 AM PDT, Brian Neradt <
>>>>>>> brian.ner...@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
>>
