Skip to content

Add config-read and run thread_birds with arguments. #265

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Markuu-s wants to merge 13 commits into
base: bird-import-proto
Choose a base branch
from
Open

Add config-read and run thread_birds with arguments. #265

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
c0cfe2f
Update release workflow
GeorgyKirichenko Aug 14, 2024
dd064b6
Export image to Dockerhub
GeorgyKirichenko Aug 22, 2024
8b9ca68
Add pragma once to config.release.h
ol-imorozko Aug 2, 2024
e50920e
Append yanet-announcer.py into YANET image
ezhk Sep 10, 2024
7c177c7
Add total counters for ingress and egress packet drops
Sep 7, 2024
c5d0822
WIP: bird import proto
GeorgyKirichenko Jun 6, 2024
8581dc1
Start bird import thread
GeorgyKirichenko Nov 22, 2024
17e89f6
Add config-read and run thread_birds with arguments.
Markuu-s Dec 28, 2024
c5aaf0a
clang-format controlplane.cpp
Markuu-s Dec 28, 2024
63e7840
Merge remote-tracking branch 'origin/main' into HEAD
Markuu-s Dec 28, 2024
a9e7fde
Fix includes.
Markuu-s Dec 28, 2024
d7ca9ba
Merge remote-tracking branch 'origin/bird-import-proto' into HEAD
Markuu-s Dec 28, 2024
2d82f7e
Fix PR.
Markuu-s Jan 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 26 additions & 80 deletions .github/workflows/release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,92 +2,38 @@ name: Release

on:
push:
tags:
- 'v[0-9]+.[0-9]+'
branch:
- 'release/*'

jobs:
build-debian-package-jammy:
name: build (Ubuntu 22.04)
runs-on: builder
container:
image: yanetplatform/builder-lite
name: build-image
runs-on: ubuntu-latest
permissions:
packages: write
contents: read
id-token: write
steps:
- uses: actions/checkout@v3
- name: Checkout
uses: actions/checkout@v4
with:
submodules: recursive
- run: |
export YANET_VERSION=${{github.ref_name}}
export YANET_VERSION=${YANET_VERSION#v}
export YANET_VERSION_MAJOR=${YANET_VERSION%.*}
export YANET_VERSION_MINOR=${YANET_VERSION#*.}
export YANET_VERSION_REVISION=${{github.run_number}}
export YANET_VERSION_HASH=${{github.sha}}
export YANET_VERSION_CUSTOM=stable
meson setup --prefix=/target \
-Dtarget=release \
-Darch=corei7,broadwell,knl \
-Dversion_major=$YANET_VERSION_MAJOR \
-Dversion_minor=$YANET_VERSION_MINOR \
-Dversion_revision=$YANET_VERSION_REVISION \
-Dversion_hash=$YANET_VERSION_HASH \
-Dversion_custom=$YANET_VERSION_CUSTOM \
build
meson compile -C build
- run: meson install -C build
- run: |
export YANET_VERSION=${{github.ref_name}}
export YANET_VERSION=${YANET_VERSION#v}
cp -r debian /target/DEBIAN
sed -i "s/__VERSION__/${YANET_VERSION}/g" /target/DEBIAN/control
- run: |
export YANET_VERSION=${{github.ref_name}}
export YANET_VERSION=${YANET_VERSION#v}
mkdir /export
dpkg-deb -b "/target" /export/yanet_${YANET_VERSION}_ubuntu22.04.deb
- uses: actions/upload-artifact@v3
- name: Docker meta
id: meta
uses: docker/metadata-action@v5
with:
name: target_debian
path: /export/yanet*.deb

build-debian-package-bionic:
name: build (Ubuntu 18.04)
runs-on: builder
container:
image: yanetplatform/builder_ubuntu18.04-lite
steps:
- uses: actions/checkout@v1
images: yanetplatform/yanet
- name: Log in to Docker Hub
uses: docker/login-action@f4ef78c080cd8ba55a85445d5b36e214a81df20a
with:
submodules: recursive
- run: |
export YANET_VERSION=${{github.ref_name}}
export YANET_VERSION=${YANET_VERSION#v}
export YANET_VERSION_MAJOR=${YANET_VERSION%.*}
export YANET_VERSION_MINOR=${YANET_VERSION#*.}
export YANET_VERSION_REVISION=${{github.run_number}}
export YANET_VERSION_HASH=${{github.sha}}
export YANET_VERSION_CUSTOM=stable
meson setup --prefix=/target \
-Dtarget=release \
-Darch=corei7,broadwell,knl \
-Dversion_major=$YANET_VERSION_MAJOR \
-Dversion_minor=$YANET_VERSION_MINOR \
-Dversion_revision=$YANET_VERSION_REVISION \
-Dversion_hash=$YANET_VERSION_HASH \
-Dversion_custom=$YANET_VERSION_CUSTOM \
build
meson compile -C build
- run: meson install -C build
- run: |
export YANET_VERSION=${{github.ref_name}}
export YANET_VERSION=${YANET_VERSION#v}
cp -r debian /target/DEBIAN
sed -i "s/__VERSION__/${YANET_VERSION}/g" /target/DEBIAN/control
- run: |
export YANET_VERSION=${{github.ref_name}}
export YANET_VERSION=${YANET_VERSION#v}
mkdir /export
dpkg-deb -b "/target" /export/yanet_${YANET_VERSION}_ubuntu18.04.deb
- uses: actions/upload-artifact@v3
username: ${{ secrets.DOCKER_USERNAME }}
password: ${{ secrets.DOCKER_PASSWORD }}
- name: Build and push Docker image
id: push
uses: docker/build-push-action@3b5e8027fcad23fda98b2e3ac259d8d67585f671
with:
name: target_debian
path: /export/yanet*.deb
context: .
file: ./build/Dockerfile.image
push: true
tags: ${{ steps.meta.outputs.tags }}
labels: ${{ steps.meta.outputs.labels }}
10 changes: 7 additions & 3 deletions build/Dockerfile.image
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
ARG RELEASE=22.04
FROM --platform=linux/amd64 ubuntu:${RELEASE} as environment
FROM --platform=linux/amd64 ubuntu:${RELEASE} AS environment

RUN apt-get update
RUN DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \
Expand Down Expand Up @@ -28,7 +28,8 @@ RUN DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \

RUN python3 -m pip install meson==0.61.2

FROM environment as builder

FROM environment AS builder

ARG YANET_VERSION_MAJOR=0
ARG YANET_VERSION_MINOR=0
Expand All @@ -50,6 +51,7 @@ RUN meson setup --prefix=/target \

RUN meson compile -C build


FROM --platform=linux/amd64 ubuntu:${RELEASE}

RUN apt-get update
Expand All @@ -61,8 +63,10 @@ RUN DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \
libmlx5-1 \
libnuma1 \
libpcap0.8 \
netbase
netbase \
python3

COPY --from=builder /opt/yanet/build/controlplane/yanet-controlplane /usr/bin/
COPY --from=builder /opt/yanet/build/dataplane/yanet-dataplane /usr/bin/
COPY --from=builder /opt/yanet/build/cli/yanet-cli /usr/bin/
COPY --from=builder /opt/yanet/yanet-announcer.py /usr/bin/yanet-announcer
9 changes: 9 additions & 0 deletions cli/telegraf.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,13 @@ void unsafe()
const auto& [memory_groups, memory_objects] = memory_stats;

const auto durations = controlplane.controlplane_durations();
uint64_t total_acl_ingress_dropPackets = 0, total_acl_egress_dropPackets = 0;

for (const auto& [coreId, worker] : responseWorkers)
{
const auto& [iterations, stats, ports_stats] = worker;
total_acl_ingress_dropPackets += stats.acl_ingress_dropPackets;
total_acl_egress_dropPackets += stats.acl_egress_dropPackets;

printf("worker,coreId=%u "
"iterations=%luu,"
Expand Down Expand Up @@ -174,6 +177,12 @@ void unsafe()
stats.logs_drops,
stats.logs_packets);

printf("worker,coreId=all "
"acl_ingress_dropPackets=%luu,"
"acl_egress_dropPackets=%luu\n",
total_acl_ingress_dropPackets,
total_acl_egress_dropPackets);

for (const auto& [physicalPortName, stats] : ports_stats)
{
printf("worker,coreId=%u,physicalPortName=%s "
Expand Down
2 changes: 2 additions & 0 deletions common/config.release.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#pragma once

#define CONFIG_YADECAP_MBUFS_COUNT (48 * 1024)
#define CONFIG_YADECAP_MBUF_SIZE (10 * 1024)
#define CONFIG_YADECAP_WORKER_PORTS_SIZE (8)
Expand Down
29 changes: 29 additions & 0 deletions common/controlplaneconfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,32 @@ class interface_t
common::globalBase::tFlow flow;
};

class bird_import_t
{
public:
void pop(common::stream_in_t& stream)
{
stream.pop(socket);
stream.pop(vrf);
stream.pop(flow);
}

void push(common::stream_out_t& stream) const
{
stream.push(socket);
stream.push(vrf);
stream.push(flow);
}

Comment thread
Markuu-s marked this conversation as resolved.
Outdated
public:
inline static const std::string socketStr = "socket";
inline static const std::string vrfStr = "vrf";
Comment on lines +114 to +115

@ol-imorozko ol-imorozko Jan 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
inline static const std::string socketStr = "socket";
inline static const std::string vrfStr = "vrf";
static constexpr auto socketStr = "socket";
static constexpr auto vrfStr = "vrf";

@Markuu-s Markuu-s Jan 24, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. "constexpr" doesn`t include "inline" in all compilers
  2. I thought about "constexpr" instead of "const" buf std::string doesn`t support it
  3. auto give us const char[N], I think in C++ it worst

@ol-imorozko ol-imorozko Feb 4, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Yeah, I think we do need inline here because we need that field to be ODR-usable: link

A static data member may be declared inline. An inline static data member can be defined in the class definition and may specify an initializer. It does not need an out-of-class definition:

struct X
{
    inline static int fully_usable = 1; // No out-of-class definition required, ODR-usable
    inline static const std::string class_name{"X"}; // Likewise
 
    static const int non_addressable = 1; // C.f. non-inline constants, usable
                                          // for its value, but not ODR-usable
    // static const std::string class_name{"X"}; // Non-integral declaration of this
                                                 // form is disallowed entirely
};

We're binding it to a reference by passing it to exist function => ODR-usage => we must provide exactly one definition of that static data member in the entire program and the simplest way to do so -- use inline here.

  1. Yeah, that's correct. Here you can use std::string_view for string literals, but the issue with it is that nlohmann::json doesn't have an overload for operator[] with std::string_view. However, it does have overloads for std::string and const char*. So you could either use the change I suggested, or go with std::string_view and pass .data() like this:
if (exist(elemJson, BirdImport::socketStr))
{
    import.socket = elemJson[BirdImport::socketStr.data()];
}

if (exist(elemJson, BirdImport::vrfStr))
{
    import.vrf = elemJson[BirdImport::vrfStr.data()];
}

Either way works for me.

  1. Why do you think so? IMHO using array types isn't inherently bad in C++. It's quite common and valid (as long as we don't decay them into pointers and/or perform pointer arithmetic :) ). I don't see any issues with using array types in this context.


std::string socket;
std::string vrf;
common::globalBase::tFlow flow;
};
Comment thread
ol-imorozko marked this conversation as resolved.

class config_t
{
public:
Expand Down Expand Up @@ -168,6 +194,7 @@ class config_t
stream.pop(local_prefixes);
stream.pop(peers);
stream.pop(interfaces);
stream.pop(bird_imports);
}

void push(common::stream_out_t& stream) const
Expand All @@ -183,6 +210,7 @@ class config_t
stream.push(local_prefixes);
stream.push(peers);
stream.push(interfaces);
stream.push(bird_imports);
}

public:
Expand All @@ -197,6 +225,7 @@ class config_t
std::set<common::ip_prefix_t> local_prefixes; ///< for fallback to default
std::map<uint32_t, std::string> peers;
std::map<std::string, interface_t> interfaces;
std::vector<bird_import_t> bird_imports;
};

}
Expand Down
4 changes: 3 additions & 1 deletion common/icp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,9 @@ using eor = std::tuple<std::string, ///< protocol
ip_address_t, ///< peer
std::string>; ///< table_name

using request = std::vector<std::variant<insert, remove, clear, eor>>;
using action = std::variant<insert, remove, clear, eor>;

using request = std::vector<action>;
}

namespace rib_summary
Expand Down
33 changes: 33 additions & 0 deletions controlplane/configparser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,32 @@ controlplane::base_t config_parser_t::loadConfig(const std::string& rootFilePath
return baseNext;
}

void config_parser_t::loadConfig_route_bird(controlplane::base_t& baseNext,
Comment thread
Markuu-s marked this conversation as resolved.
Outdated
std::vector<controlplane::route::bird_import_t>& birdsImport,
const nlohmann::json& birdJson)
{
using BirdImport = controlplane::route::bird_import_t;
for (const auto& elemJson : birdJson)
{
BirdImport import;

if (exist(elemJson, BirdImport::socketStr))
{
import.socket = elemJson[BirdImport::socketStr];
}

if (exist(elemJson, BirdImport::vrfStr))
{
import.vrf = elemJson[BirdImport::vrfStr];
}
Comment on lines +209 to +217

@ol-imorozko ol-imorozko Feb 4, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if those keys do not exist? We need to use some default value then, right? Otherwise I think it will be two empty strings.
It's better to use value(const typename object_t::key_type& key, const ValueType& default_value) method of nlohmann::json like so:

Suggested change
if (exist(elemJson, BirdImport::socketStr))
{
import.socket = elemJson[BirdImport::socketStr];
}
if (exist(elemJson, BirdImport::vrfStr))
{
import.vrf = elemJson[BirdImport::vrfStr];
}
import.socket = elemJson.value(BirdImport::socketStr, "some_default_value");
import.vrf = elemJson.value(BirdImport::vrfStr, "some_default_value");


birdsImport.push_back(import);
YANET_LOG_INFO("loadConfig_route_bird: socket(%s), vrf(%s)\n",
import.socket.data(),
import.vrf.data());
}
Comment thread
Markuu-s marked this conversation as resolved.
Outdated
}

void config_parser_t::loadConfig_logicalPort(controlplane::base_t& baseNext,
const std::string& moduleId,
const nlohmann::json& moduleJson)
Expand Down Expand Up @@ -367,6 +393,13 @@ void config_parser_t::loadConfig_route(controlplane::base_t& baseNext,
route.tunnel_enabled = false;
}

if (exist(moduleJson, "birdImport"))
{
loadConfig_route_bird(baseNext,
route.bird_imports,
moduleJson["birdImport"]);
}

//

route.routeId = routeId;
Expand Down
1 change: 1 addition & 0 deletions controlplane/configparser.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class config_parser_t
void loadConfig_logicalPort(controlplane::base_t& baseNext, const std::string& moduleId, const nlohmann::json& moduleJson);
void loadConfig_route(controlplane::base_t& baseNext, const std::string& moduleId, const nlohmann::json& moduleJson, const std::string& rootFilePath, const std::map<std::string, nlohmann::json>& jsons);
void loadConfig_route_peers(controlplane::base_t& baseNext, controlplane::route::config_t& route, const nlohmann::json& json, const std::string& rootFilePath, const std::map<std::string, nlohmann::json>& jsons);
void loadConfig_route_bird(controlplane::base_t& baseNext, std::vector<controlplane::route::bird_import_t>& birdsImport, const nlohmann::json& birdJson);
void loadConfig_decap(controlplane::base_t& baseNext, const std::string& moduleId, const nlohmann::json& moduleJson);
void loadConfig_nat64stateful(controlplane::base_t& baseNext, const std::string& moduleId, const nlohmann::json& moduleJson, const std::string& rootFilePath, const std::map<std::string, nlohmann::json>& jsons);
void loadConfig_nat64stateless(controlplane::base_t& baseNext, const std::string& moduleId, const nlohmann::json& moduleJson, const std::string& rootFilePath, const std::map<std::string, nlohmann::json>& jsons);
Expand Down
18 changes: 18 additions & 0 deletions controlplane/controlplane.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,18 @@ common::icp::controlplane_values::response cControlPlane::controlplane_values()
return response;
}

common::icp::route_config::response cControlPlane::getRoute() const
{
common::icp::route_config::response response;

{
auto current_guard = generations.current_lock_guard();
response = generations.current().routes;
}

return response;
}

common::icp::getDecapPrefixes::response cControlPlane::command_getDecapPrefixes()
{
common::icp::getDecapPrefixes::response response;
Expand Down Expand Up @@ -924,6 +936,12 @@ eResult cControlPlane::loadConfig(const std::string& rootFilePath,
}

YANET_LOG_INFO("dataplane has been updated (stage 7)\n");
for(auto& module : modules) {
if (rib_t* rib = dynamic_cast<rib_t*>(module)) {
rib->bird_import_get();
rib->moduleStart();
}
}
}
Comment on lines +956 to 964

@ol-imorozko ol-imorozko Jan 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is every module is also a rib module? I don't quite understand.
Also, dynamic_cast will inevitably lead to some errors, if, for example, we decide to change class hierarchy.
Can we use other things here? Like maybe we can utilize one or two virtual functions?

@Markuu-s Markuu-s Jan 24, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this stage, I need to call one method that is only in rib_t (bird_import_get()), using a virtual function for all classes is somehow strange. I'll think about it, or I'm ready to discuss what's best.

@ol-imorozko ol-imorozko Feb 4, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modules is a vector of cModule pointers. So what you can do is add a new field to the class that will point to the required module, and here just call it's methods.
Something like an iterator to the modules will suffice (but be aware of invalidation). Or just a pointer/reference to rib_t.
Either way it's better than what we have how.

catch (const error_result_t& error)
{
Expand Down
1 change: 1 addition & 0 deletions controlplane/controlplane.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ class cControlPlane
common::icp::acl_unwind::response acl_unwind(const common::icp::acl_unwind::request& request) const;
common::icp::acl_lookup::response acl_lookup(const common::icp::acl_lookup::request& request) const;
common::icp::controlplane_values::response controlplane_values() const;
common::icp::route_config::response getRoute() const;

common::icp::getDecapPrefixes::response command_getDecapPrefixes();
common::icp::getNat64statelessTranslations::response command_getNat64statelessTranslations();
Expand Down
Loading