On Sat, Jun 10, 2017 at 10:31 PM, Jamal Hadi Salim <j...@mojatatu.com> wrote: > skb->priority = sk->sk_priority; > - skb->mark = sk->sk_mark; > + if (!skb->mark) > + skb->mark = sk->sk_mark;
It looks a bit iffy to take sk->sk_mark only if skb->mark is zero instead of relying on the callers to tell this function what they want. I think the patch is correct, but it might be better to fix the other callers (dccp_make_response and dccp_ctl_make_reset) to set skb->mark to what they want. Either way. > tcp_ecn_make_synack(req, th); > th->source = htons(ireq->ir_num); > th->dest = ireq->ir_rmt_port; > + if (sock_net(sk)->ipv4.sysctl_tcp_fwmark_accept) > + skb->mark = ireq->ir_mark; I think checking the sysctl here is unnecessary. It seems to me that ir_mark already takes that into account. Its semantics (see inet_request_mark) are: - If listen socket has a nonzero mark, use that - Else if sysctl_tcp_fwmark_accept is set and inbound SYN packet has mark, use that - Else zero. which is what you want. Other than that, Reviewed-By: Lorenzo Colitti <lore...@google.com> Please disregard my earlier comment about fwmark_reflect - I didn't notice that the code sets ir_mark based on tcp_fwmark_accept, and doesn't look at fwmark_reflect at all.
