Cake - FQ_codel the next generation
 help / color / mirror / Atom feed
From: <gamanakis@gmail.com>
To: "'Toke Høiland-Jørgensen'" <toke@toke.dk>,
	"'Jonathan Morton'" <chromatix99@gmail.com>
Cc: <Cake@lists.bufferbloat.net>
Subject: [Cake]  host hashes and NAT neglected in src/dst-host mode
Date: Sun, 6 Jan 2019 23:00:11 -0500	[thread overview]
Message-ID: <008901d4a63d$7d054420$770fcc60$@gmail.com> (raw)

I was having a look at cake's hash code and it seems srchost_hash and
dsthost_hash, as well as the flowkeys in NAT are not calculated in plain
"srchost" or "dsthost" modes. CAKE_FLOW_FLOWS and CAKE_FLOW_HOSTS are both 0
(conditional in line 633) when src/dst-host is set as flow mode. So the code
goes to "skip_hash" (line 683), and as a result the flowkeys are not updated
in respect to NAT (line 640), but also srchost_hash and dsthost_hash (line
653) are not calculated. 

Is this intentional? I would expect the NAT flowkeys to be updated in
src/dst-host modes, and also the host hashes to be calculated for fair host
isolation.

cake_hash() in sch_cake.c from linux-4.20:

 632         /* If both overrides are set we can skip packet dissection
entirely */
 633         if ((flow_override || !(flow_mode & CAKE_FLOW_FLOWS)) &&
 634             (host_override || !(flow_mode & CAKE_FLOW_HOSTS)))
 635                 goto skip_hash;
 636
 637         skb_flow_dissect_flow_keys(skb, &keys,
 638
FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL);
 639
 640         if (flow_mode & CAKE_FLOW_NAT_FLAG)
 641                 cake_update_flowkeys(&keys, skb);
 642
 643         /* flow_hash_from_keys() sorts the addresses by value, so we
have
 644          * to preserve their order in a separate data structure to
treat
 645          * src and dst host addresses as independently selectable.
 646          */
 647         host_keys = keys;
 648         host_keys.ports.ports     = 0;
 649         host_keys.basic.ip_proto  = 0;
 650         host_keys.keyid.keyid     = 0;
 651         host_keys.tags.flow_label = 0;
 652
 653         switch (host_keys.control.addr_type) {
 654         case FLOW_DISSECTOR_KEY_IPV4_ADDRS:
 655                 host_keys.addrs.v4addrs.src = 0;
 656                 dsthost_hash = flow_hash_from_keys(&host_keys);
 657                 host_keys.addrs.v4addrs.src = keys.addrs.v4addrs.src;
 658                 host_keys.addrs.v4addrs.dst = 0;
 659                 srchost_hash = flow_hash_from_keys(&host_keys);
 660                 break;
 661
 662         case FLOW_DISSECTOR_KEY_IPV6_ADDRS:
 663                 memset(&host_keys.addrs.v6addrs.src, 0,
 664                        sizeof(host_keys.addrs.v6addrs.src));
 665                 dsthost_hash = flow_hash_from_keys(&host_keys);
 666                 host_keys.addrs.v6addrs.src = keys.addrs.v6addrs.src;
 667                 memset(&host_keys.addrs.v6addrs.dst, 0,
 668                        sizeof(host_keys.addrs.v6addrs.dst));
 669                 srchost_hash = flow_hash_from_keys(&host_keys);
 670                 break;
 671
 672         default:
 673                 dsthost_hash = 0;
 674                 srchost_hash = 0;
 675         }
 676
 677         /* This *must* be after the above switch, since as a
 678          * side-effect it sorts the src and dst addresses.
 679          */
 680         if (flow_mode & CAKE_FLOW_FLOWS)
 681                 flow_hash = flow_hash_from_keys(&keys);
 682
 683 skip_hash:
 684         if (flow_override)
 685                 flow_hash = flow_override - 1;
 686         if (host_override) {
 687                 dsthost_hash = host_override - 1;
 688                 srchost_hash = host_override - 1;
 689         }
 690
 691         if (!(flow_mode & CAKE_FLOW_FLOWS)) {
 692                 if (flow_mode & CAKE_FLOW_SRC_IP)
 693                         flow_hash ^= srchost_hash;
 694
 695                 if (flow_mode & CAKE_FLOW_DST_IP)
 696                         flow_hash ^= dsthost_hash;
 697         }
 698
 699         reduced_hash = flow_hash % CAKE_QUEUES;





             reply	other threads:[~2019-01-07  4:00 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-07  4:00 gamanakis [this message]
2019-01-07  8:20 ` Jonathan Morton
2019-01-07 15:11   ` Georgios Amanakis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://lists.bufferbloat.net/postorius/lists/cake.lists.bufferbloat.net/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='008901d4a63d$7d054420$770fcc60$@gmail.com' \
    --to=gamanakis@gmail.com \
    --cc=Cake@lists.bufferbloat.net \
    --cc=chromatix99@gmail.com \
    --cc=toke@toke.dk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox