Xiao Liang <shaw.leon@xxxxxxxxx> wrote:
> Since proto_th carries no information about the proto number, we need to
> preserve the L4 protocol expression.
>
> For example, if "meta l4proto 91 @th,0,16 0" is simplified to
> "th sport 0", the information of protocol number is lost. This patch
> changes it to "meta l4proto 91 th sport 0".
>
> Signed-off-by: Xiao Liang <shaw.leon@xxxxxxxxx>
> ---
>
> Technically, if port is not defined for the L4 protocol, it's better to
> keep "@th,0,16" as raw payload expressions rather than "th sport". But
> it's not easy to figure out the context.
Yes, this is also because we don't store dependencies like
'meta l4proto { tcp, udp }', so we don't have access to this info when
we see the @th,0,16 load.
What do you make of this (it will display @th syntax for your test case)?
diff --git a/src/payload.c b/src/payload.c
--- a/src/payload.c
+++ b/src/payload.c
@@ -851,6 +851,58 @@ static bool payload_may_dependency_kill_ll(struct payload_dep_ctx *ctx, struct e
return true;
}
+static bool l4proto_has_ports(struct payload_dep_ctx *ctx, struct expr *expr)
+{
+ uint8_t v;
+
+ assert(expr->etype == EXPR_VALUE);
+
+ if (expr->len != 8)
+ return false;
+
+ v = mpz_get_uint8(expr->value);
+
+ switch (v) {
+ case IPPROTO_UDPLITE:
+ case IPPROTO_UDP:
+ case IPPROTO_TCP:
+ case IPPROTO_DCCP:
+ case IPPROTO_SCTP:
+ return true;
+ }
+
+ return false;
+}
+
+static bool payload_may_dependency_kill_th(struct payload_dep_ctx *ctx, struct expr *expr)
+{
+ struct expr *dep = payload_dependency_get(ctx, expr->payload.base);
+ enum proto_bases b;
+
+ switch (dep->left->etype) {
+ case EXPR_PAYLOAD:
+ b = dep->left->payload.base;
+ break;
+ case EXPR_META:
+ b = dep->left->meta.base;
+ break;
+ default:
+ return false;
+ }
+
+ if (b != PROTO_BASE_NETWORK_HDR)
+ return false;
+
+ switch (dep->right->etype) {
+ case EXPR_VALUE:
+ return l4proto_has_ports(ctx, dep->right);
+ default:
+ break;
+ }
+
+ return false;
+}
+
static bool payload_may_dependency_kill(struct payload_dep_ctx *ctx,
unsigned int family, struct expr *expr)
{
@@ -893,6 +945,15 @@ static bool payload_may_dependency_kill(struct payload_dep_ctx *ctx,
if (expr->payload.base != PROTO_BASE_TRANSPORT_HDR)
return true;
+ if (expr->payload.desc == &proto_th) {
+ if (payload_may_dependency_kill_th(ctx, expr))
+ return true;
+
+ /* prefer @th syntax, we don't have a 'source/destination port' protocol */
+ expr->payload.desc = &proto_unknown;
+ return false;
+ }
+
if (dep->left->etype != EXPR_PAYLOAD ||
dep->left->payload.base != PROTO_BASE_TRANSPORT_HDR)
return true;
It would make sense to NOT set &proto_th from payload_init_raw(), but
that would place significant burden on netlink_delinearize to
pretty-print the typical use case for this, i.e.
'meta l4proto { tcp, udp } th dport 53 accept'
