Sorry for the latency.
On 17-06-13 09:58 AM, Lorenzo Colitti wrote:
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.
I saw the DCCP call - but the systcl says "tcp" on it and the feature
is not used by dccp currently. i.e it looked like an unreasonably large
change to update ip_build_and_send_pkt() params in particular when
the skb already had the mark.
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.
I see it. I'll fix this part in next version.
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.
np.
cheers,
jamal
