Thanks for working on this David.

On Sat, Dec 15, 2018 at 9:39 AM David Marchand <[email protected]>
wrote:

> The ftp alg relies on the attached nat information to the current
> connection to trigger the nat operation while it should take the
> information from the rule being evaluated.
>

I cannot connect the commit message with the code change.
Could you add a test case, so I might understand ?



>
> Signed-off-by: David Marchand <[email protected]>
> ---
>  lib/conntrack.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index d08d0ea..41c56c1 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -3204,7 +3204,7 @@ handle_ftp_ctl(struct conntrack *ct, const struct
> conn_lookup_ctx *ctx,
>              VLOG_WARN_RL(&rl, "Invalid FTP control packet format");
>              pkt->md.ct_state |= CS_TRACKED | CS_INVALID;
>              return;
> -        } else if (rc == CT_FTP_CTL_INTEREST) {
> +        } else if (rc == CT_FTP_CTL_INTEREST && nat) {
>

This line tries to defeat the original intent of the code towards common
code path and exercising.



>              uint16_t ip_len;
>              int64_t new_skew;
>
> @@ -3232,7 +3232,7 @@ handle_ftp_ctl(struct conntrack *ct, const struct
> conn_lookup_ctx *ctx,
>                                        new_skew + seq_skew, ctx->reply);
>                  }
>              }
> -        } else {
> +        } else if (rc == CT_FTP_CTL_OTHER) {
>

This line just compensates for the first line change to avoid hitting the
assert sanity check.



>              OVS_NOT_REACHED();
>          }
>      } else if (ftp_ctl == CT_FTP_CTL_INVALID) {
> --
> 1.8.3.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to