Re: [PATCH iproute2-next v2] ip-xfrm: Add support for OUTPUT_MARK
On 6/13/18 11:09 PM, Subash Abhinov Kasiviswanathan wrote: > The output mark differs from the existing xfrm mark in two ways: > > 1. The xfrm mark is used to match xfrm policies and states, while > the xfrm output mark is used to set the mark (and influence > the routing) of the packets emitted by those states. > 2. The existing mark is constrained to be a subset of the bits of > the originating socket or transformed packet, but the output > mark is arbitrary and depends only on the state. make sure the man page update contains the description of the 2 marks and why they are used. Thanks,
Re: [PATCH iproute2-next v2] ip-xfrm: Add support for OUTPUT_MARK
any reason to put output-mark on its own line? Why not mark 0x1/0x3 output-mark 0x2 Hi David I will move it to the same line in v3. is the documentation clear on the difference between mark and output-mark? Lorenzo has described the differences in detail in the kernel commit I had listed in the commit text of this patch. Pasting from there - The output mark differs from the existing xfrm mark in two ways: 1. The xfrm mark is used to match xfrm policies and states, while the xfrm output mark is used to set the mark (and influence the routing) of the packets emitted by those states. 2. The existing mark is constrained to be a subset of the bits of the originating socket or transformed packet, but the output mark is arbitrary and depends only on the state. The use of a separate mark provides additional flexibility. For example: - A packet subject to two transforms (e.g., transport mode inside tunnel mode) can have two different output marks applied to it, one for the transport mode SA and one for the tunnel mode SA. - On a system where socket marks determine routing, the packets emitted by an IPsec tunnel can be routed based on a mark that is determined by the tunnel, not by the marks of the unencrypted packets. - Support for setting the output marks can be introduced without breaking any existing setups that employ both mark-based routing and xfrm tunnel mode. Simply changing the code to use the xfrm mark for routing output packets could xfrm mark could change behaviour in a way that breaks these setups. @@ -61,6 +61,7 @@ static void usage(void) fprintf(stderr, "[ flag FLAG-LIST ] [ sel SELECTOR ] [ LIMIT-LIST ] [ encap ENCAP ]\n"); fprintf(stderr, "[ coa ADDR[/PLEN] ] [ ctx CTX ] [ extra-flag EXTRA-FLAG-LIST ]\n"); fprintf(stderr, "[ offload [dev DEV] dir DIR ]\n"); + fprintf(stderr, "[ output-mark OUTPUT-MARK]\n"); Nit: I think you want a space between OUTPUT-MARK and ]. yes. I will update this as well. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH iproute2-next v2] ip-xfrm: Add support for OUTPUT_MARK
On 6/12/18 9:14 PM, Lorenzo Colitti wrote: > On Wed, Jun 13, 2018 at 3:48 AM Subash Abhinov Kasiviswanathan > wrote: >> >> src 192.168.1.1 dst 192.168.1.2 >> proto esp spi 0x4321 reqid 0 mode tunnel >> replay-window 0 flag af-unspec >> mark 0x1/0x3 >> output-mark 0x2 > > Nit: I don't know what guarantees we provide (if any) that the output > format of "ip xfrm state" does not change except to add new lines at > the end. Personally, I feel that an app or script that depends on > "auth-trunc" (or anything else, really) being on the line immediately > after "mark" is brittle and should be fixed. This is particularly true > since in general between the mark and the encryption there might be an > auth-trunc line, or an auth line, or neither. As such, adding this > line here seems OK to me. any reason to put output-mark on its own line? Why not mark 0x1/0x3 output-mark 0x2 is the documentation clear on the difference between mark and output-mark? > >> @@ -61,6 +61,7 @@ static void usage(void) >> fprintf(stderr, "[ flag FLAG-LIST ] [ sel SELECTOR ] [ >> LIMIT-LIST ] [ encap ENCAP ]\n"); >> fprintf(stderr, "[ coa ADDR[/PLEN] ] [ ctx CTX ] [ >> extra-flag EXTRA-FLAG-LIST ]\n"); >> fprintf(stderr, "[ offload [dev DEV] dir DIR ]\n"); >> + fprintf(stderr, "[ output-mark OUTPUT-MARK]\n"); > > Nit: I think you want a space between OUTPUT-MARK and ]. yes. > > Other than that, > > Acked-by: Lorenzo Colitti >
Re: [PATCH iproute2-next v2] ip-xfrm: Add support for OUTPUT_MARK
On Wed, 13 Jun 2018 12:14:53 +0900 Lorenzo Colitti wrote: > On Wed, Jun 13, 2018 at 3:48 AM Subash Abhinov Kasiviswanathan > wrote: > > > > src 192.168.1.1 dst 192.168.1.2 > > proto esp spi 0x4321 reqid 0 mode tunnel > > replay-window 0 flag af-unspec > > mark 0x1/0x3 > > output-mark 0x2 > > Nit: I don't know what guarantees we provide (if any) that the output > format of "ip xfrm state" does not change except to add new lines at > the end. Personally, I feel that an app or script that depends on > "auth-trunc" (or anything else, really) being on the line immediately > after "mark" is brittle and should be fixed. This is particularly true > since in general between the mark and the encryption there might be an > auth-trunc line, or an auth line, or neither. As such, adding this > line here seems OK to me. Scripts should use json mode. If it ever gets added to xfrm output (hint).
Re: [PATCH iproute2-next v2] ip-xfrm: Add support for OUTPUT_MARK
On Wed, Jun 13, 2018 at 3:48 AM Subash Abhinov Kasiviswanathan wrote: > > src 192.168.1.1 dst 192.168.1.2 > proto esp spi 0x4321 reqid 0 mode tunnel > replay-window 0 flag af-unspec > mark 0x1/0x3 > output-mark 0x2 Nit: I don't know what guarantees we provide (if any) that the output format of "ip xfrm state" does not change except to add new lines at the end. Personally, I feel that an app or script that depends on "auth-trunc" (or anything else, really) being on the line immediately after "mark" is brittle and should be fixed. This is particularly true since in general between the mark and the encryption there might be an auth-trunc line, or an auth line, or neither. As such, adding this line here seems OK to me. > @@ -61,6 +61,7 @@ static void usage(void) > fprintf(stderr, "[ flag FLAG-LIST ] [ sel SELECTOR ] [ > LIMIT-LIST ] [ encap ENCAP ]\n"); > fprintf(stderr, "[ coa ADDR[/PLEN] ] [ ctx CTX ] [ extra-flag > EXTRA-FLAG-LIST ]\n"); > fprintf(stderr, "[ offload [dev DEV] dir DIR ]\n"); > + fprintf(stderr, "[ output-mark OUTPUT-MARK]\n"); Nit: I think you want a space between OUTPUT-MARK and ]. Other than that, Acked-by: Lorenzo Colitti