Hi Andrea,

Thank you for the review. The points shared with patches 1-3 will be
addressed as described in those replies; below the
End.M.GTP6.D-specific ones.

The "src" attribute is used verbatim here as the outer IPv6 source address,
same as patch 3. The src dual-semantics overload flagged in the patch 3
reply applies here too.

Covered in the patch 3 reply: with the End.M.GTP4.E template use
gone, verbatim outer IPv6 SA becomes the single meaning of the src
attribute for the IPv6-emitting behaviors.

Thank you for the follow-up in the cover letter thread. The finish callback
writes orig_dst into SRH[0] and Args.Mob.Session into SRH[1]. As far as I
can see, this matches neither Section 6.3 (Args.Mob.Session in SRH[0], no
D) nor Section 6.4 (D in SRH[0], no Args.Mob).

Confirmed, that is the bug from my May 10 note. The next version of
End.M.GTP6.D will push the configured SR Policy verbatim and stamp
Args.Mob.Session into SRH[0] (at the locator length given by the
explicit sr_prefix_len attribute) per Section 6.3 S08; preserving the
original outer DA in a prepended slot will be exclusive to
End.M.GTP6.D.Di.

Same reverse Christmas tree as patch 2; same issue in the other functions
introduced by this patch.
gtp is only used as a cast intermediary. Could it be inlined?

Will fix both.

Nit: gtphl and hdrlen are assigned before the GTP1_F_EXTHDR check. On the
path where the E flag is not set, gtphl is unused. Moving the gtphl
assignment after the check would make the flow clearer.

Will move the gtphl dereference after the check; the pull has to stay
before it, since the long header is also consumed for S/PN-only
flags.

Maybe ext could be renamed to ext_hdr? It would be easier to distinguish
from ext_units and ext_bytes.
ext_units is only used to derive ext_bytes. A single ext_len would
remove the intermediate variable.

Will do both.

If the extension chain contains more than one PDU Session Container, *qfi
is silently overwritten. Is that intentional, or should the function reject
a duplicate?

Not intentional; will reject a duplicate PDU Session Container as
malformed, with a selftest case for it.

ext[ext_bytes - 1] reads the Next Extension Header Type field from the last
byte of the current extension. Would a short comment help the reader?

Will add one.

input_action_end_m_gtp6_d() does not change skb_dst(skb) before this call,
so dst and lwtstate are the same ones the caller already dereferenced. When
can this NULL check trigger?

It cannot: for a route installed with LWTUNNEL_STATE_INPUT_REDIRECT,
lwtunnel_set_redirect() always populates orig_input before dst.input
is replaced. I will drop the checks and call orig_input directly.

Same dst/lwtstate issue as patch 2. Not introduced by this patch.
Same missing iptunnel_handle_offloads() as patch 2.

The NF_HOOK split goes away per the cover letter thread, and the SRv6
push will go through a shared helper that calls
iptunnel_handle_offloads(skb, SKB_GSO_IPXIP6) before
seg6_do_srh_encap().

Same BAD_INNER misuse as patch 2. seg6_do_srh_encap() can also fail from
seg6_push_hmac(), which is an HMAC error on the new SRH, not an inner-T-PDU
problem.

[...]

segments[0], segments[1], saddr, and daddr are written after
seg6_do_srh_encap() already called skb_postpush_rcsum(). skb->csum can
be stale. Same for any later change to the outer header or SRH.

HMAC, if configured, is computed on non-final SRH and saddr, hence invalid.

Thanks, both of these are real issues. My plan for the next version:

- every field stamped after seg6_do_srh_encap() (Args.Mob.Session, the
  preserved DA in the drop-in variant, the outer saddr/daddr refresh,
  and the dsfield propagation in H.M.GTP4.D) will go through a small
  helper that applies the corresponding diff to skb->csum when the skb
  is CHECKSUM_COMPLETE;

- the D-side behaviors will reject an HMAC-flagged SRH template at
  configuration time: stamping the per-packet fields after
  seg6_do_srh_encap() has signed the SRH would always invalidate the
  HMAC. Inbound HMAC validation is unaffected. Would you prefer the
  stamp-before-sign ordering solved from the start instead?

The initializer on reason is dead. Every goto drop path sets reason
explicitly before the jump. The variable can be left uninitialized here.

This goes away with the drop-reason rework: the MUP drop reasons will
be out of the initial series per the prep series plan, so the variable
itself is removed.

Same SRH validation concerns as patch 1. HMAC is not validated here.

The ingress will use the same three-state SRH helper as the other
behaviors, which validates the HMAC whenever an SRH is present.

Limitation note for both input_action_end() calls above: correct per RFC
9433 Section 6.3 S10-S11, but the SRH is absent or SL == 0 here, so
input_action_end() will always drop without signaling non-GTP-U traffic.
Perhaps you meant to drop directly with BAD_GTPU?

Right, the End fallback could only ever drop here. Instead of
dropping, I plan to hand non-UDP, non-GTP-U and non-T-PDU packets to
the route's original input path (the orig_input saved by the lwtunnel
input redirect), so a downstream owner of the GTP-U control plane
still receives e.g. Echo Request; the selftests will cover that
passthrough.

Nit: inner_first could be an inner_ver with the shift done at assignment.
The name would say what the variable holds.

[...]

Same repeated size-selection ternary as patch 2.

Will do both: the inner version, header length and protocol computed
once in the switch.

The anonymous { } block scopes three variables that should be declared at
function top. Splitting into smaller helpers would make this easier to
follow.

Will split the dispatch and outer strip into a decap helper shared
with End.M.GTP6.D.Di, with declarations at function top.

Same missing frag_off check as patch 2.

Will add.

The "{,.Di}" shell brace notation is unusual. Emitting the actual
behavior name (End.M.GTP6.D or End.M.GTP6.D.Di) would be clearer.
Same applies wherever this notation appears in the patchset.

Will replace it with the concrete behavior name everywhere.

Thanks,
Yuya