Hi Alin,
On Wed, Dec 12, 2018 at 05:56:46PM +0100, Alin Năstac wrote:
[...]
> > One thing we could probably do is to add nf_nat_rtp_rtcp_expected()
> > instead of using the generic nf_nat_sip_expected() which is also
> > called for the REGISTER case. I think we only need this code below for
> > the RTP/RTCP case as you explained.
>
> That's true, but signalling expectations do not have a paired
> expectation anyway.
> If you want to exclude this logic for them, it is enough to condition
> it with exp->helper != nfct_help(ct)->helper.
>
> What variant do you prefer?
Restricting the helper should be fine too, so we don't need a new
function. Probably you can check for the expectation class instead?
> > > > > > + range.flags = (NF_NAT_RANGE_MAP_IPS |
> > > > > > NF_NAT_RANGE_PROTO_SPECIFIED);
> > > > > > + range.min_proto.all = range.max_proto.all =
> > > > > > pair_exp->tuple.dst.u.all;
> > > > > > + range.min_addr = range.max_addr =
> > > > > > pair_exp->tuple.dst.u3;
> > > > > > + range_set_for_snat = 1;
I can see in the existing code:
#1 do DST manip case: so range.flags, .min_proto and .min_addr are set.
#2 In case of paired expectation in place, we overwrite these range fields
again.
#3 No override for the _SRC case.
If your patch can help disentagle this code a bit by:
#1 check for paired expectation, if so do handling, return.
#2 check for _SRC manip needed, return.
#3 existing _DST manip case.
It may be larger patch, but we skip 'range_set_for_snat'?
More comments below.
> > > > > > + break;
> > > > > > + }
> > > > > > + }
> > > > > > + spin_unlock_bh(&nf_conntrack_expect_lock);
> > > > > > +
> > > > > > + /* When no paired expected conntrack has been found, change
> > > > > > src to
> > > > > > + * where master sends to, but only if the connection actually
> > > > > > came
> > > > > > + * from the same source. */
For new comments, we prefer this format:
/* When no paired expected conntrack has been found, change src
to
* where master sends to, but only if the connection actually
came
* from the same source.
*/
Even if many old spots still use the old format.
And copy and paste the long description you made in your previous
email in the commit log, it's good for the record :-).
Thanks a lot for your patience !
