Copilot commented on code in PR #13412:
URL: https://github.com/apache/trafficserver/pull/13412#discussion_r3623597313
##########
include/ts/ts.h:
##########
@@ -1601,6 +1601,32 @@ TSReturnCode TSHttpSsnClientFdGet(TSHttpSsn
ssnp, int *fdp);
*/
TSReturnCode TSHttpTxnClientPacketMarkSet(TSHttpTxn txnp, int mark);
+/** Change selected bits of the packet firewall mark for the client side
connection
+
+ Only the bits selected by @a mask are modified. Bits set in @a mask are
updated from the
+ corresponding bits of @a mark; bits clear in @a mask retain their previous
value. Equivalently,
+ the resulting mark is computed as:
+
+ @code
+ result = (current_mark & ~mask) | (mark & mask);
+ @endcode
+
+ @a mark and @a mask are interpreted as 32-bit unsigned bit patterns. A @a
mask of 0xFFFFFFFF
+ replaces the entire mark and is equivalent to the two-argument overload. A
@a mask of 0 leaves
+ the mark unchanged.
+
+ @note The firewall mark is only honored on platforms whose OS supports it,
specifically Linux via
+ @c SO_MARK. On platforms without @c SO_MARK support the call still returns
TS_SUCCESS when a
+ client connection is present, but setting the mark has no effect at the OS
layer (it is a safe
+ no-op).
+
+ @note The change takes effect immediately on the live client connection
+
+ @return TS_SUCCESS if the client connection was modified, TS_ERROR if
there is no client
+ connection to modify
+*/
+TSReturnCode TSHttpTxnClientPacketMarkSet(TSHttpTxn txnp, int mark, int mask);
Review Comment:
Adding a new overload for TSHttpTxnClientPacketMarkSet introduces an API
source-compatibility break for C++ plugins that take the function address or
pass it as a template argument without an explicit cast (the name becomes an
overload set and can become ambiguous). To preserve source compatibility for
existing plugins, consider using a distinct function name for the masked
variant (e.g. TSHttpTxnClientPacketMarkSetMasked) instead of overloading, or
providing a non-overloaded named entry point.
##########
include/ts/ts.h:
##########
@@ -1617,6 +1643,32 @@ TSReturnCode TSHttpTxnClientPacketMarkSet(TSHttpTxn
txnp, int mark);
*/
TSReturnCode TSHttpTxnServerPacketMarkSet(TSHttpTxn txnp, int mark);
+/** Change selected bits of the packet firewall mark for the server side
connection
+
+ Only the bits selected by @a mask are modified. Bits set in @a mask are
updated from the
+ corresponding bits of @a mark; bits clear in @a mask retain their previous
value. Equivalently,
+ the resulting mark is computed as:
+
+ @code
+ result = (current_mark & ~mask) | (mark & mask);
+ @endcode
+
+ @a mark and @a mask are interpreted as 32-bit unsigned bit patterns. A @a
mask of 0xFFFFFFFF
+ replaces the entire mark and is equivalent to the two-argument overload. A
@a mask of 0 leaves
+ the mark unchanged. The current mark in this formula is the value recorded
on @a txnp, not
+ socket state.
+
+ @note The firewall mark is only honored on platforms whose OS supports it,
specifically Linux via
+ @c SO_MARK. On platforms without @c SO_MARK support the call still returns
TS_SUCCESS, but setting
+ the mark has no effect at the OS layer (it is a safe no-op).
+
+ @note If a live server connection exists, the mark is applied to it
immediately; the mark is also
+ recorded on the transaction so that any subsequent server connection for
this transaction uses it.
+
+ @return TS_SUCCESS always, including when no server connection has been
established yet.
+*/
+TSReturnCode TSHttpTxnServerPacketMarkSet(TSHttpTxn txnp, int mark, int mask);
Review Comment:
Same overloading concern as the client-side API: adding a new overload for
TSHttpTxnServerPacketMarkSet can break existing plugin code that references
TSHttpTxnServerPacketMarkSet as a single function (e.g. assigning to a
variable) because the identifier becomes an overload set. Consider a distinct
function name for the masked variant to avoid that compatibility issue.
##########
tests/gold_tests/pluginTest/packet_mark/packet_mark.test.py:
##########
@@ -175,7 +235,46 @@ def run(self):
self._add_case(self.ECHO_HEADER, "server_packet_mark sets the
server-side mark on the live connection", "X-Set-Mark")
self._add_case(
self.ECHO_HEADER, "server_packet_mark seeds the mark for a future
origin connection", "X-Set-Mark-Preconnect")
Review Comment:
The server-side plugin now prefers the masked overload in both the
live-connection path and the preconnect (no origin connection yet) path, but
the AuTest only exercises masked updates for X-Set-Mark (live connection). Add
a masked test case for the X-Set-Mark-Preconnect + X-Set-Mask combination to
validate the "compose against recorded transaction config" behavior before an
origin connection exists.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
