Skip to content

Commit 0132d27

Browse files
authored
Refactors the entire open overloads to delegate from less to more parametetarized versions. (#1995)
- Renamed initConfig to populateConfigDefaults so it only updates the config and does not write to the member variable config. - Fixed bug where open(XdpDeviceConfigration) overload replaced the configuration before checking if the device is open.
1 parent a01e705 commit 0132d27

2 files changed

Lines changed: 34 additions & 36 deletions

File tree

Pcap++/header/XdpDevice.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
/// @file
44

55
#include "Device.h"
6+
#include <memory>
67
#include <utility>
78
#include <functional>
89

@@ -230,7 +231,8 @@ namespace pcpp
230231
/// @return A pointer to the current device configuration. If the device is not open this method returns nullptr
231232
XdpDeviceConfiguration* getConfig() const
232233
{
233-
return m_Config;
234+
// TODO: Return a copy or const ref to avoid user modifying config?
235+
return m_Config.get();
234236
}
235237

236238
/// @return Current device statistics
@@ -293,7 +295,7 @@ namespace pcpp
293295
bool m_DeviceOpened = false;
294296

295297
std::string m_InterfaceName;
296-
XdpDeviceConfiguration* m_Config;
298+
std::unique_ptr<XdpDeviceConfiguration> m_Config;
297299
bool m_ReceivingPackets;
298300
XdpUmem* m_Umem;
299301
void* m_SocketInfo;
@@ -308,7 +310,7 @@ namespace pcpp
308310
uint32_t checkCompletionRing();
309311
bool configureSocket();
310312
bool initUmem();
311-
bool initConfig();
313+
bool populateConfigDefaults(XdpDeviceConfiguration& config) const;
312314
bool getSocketStats();
313315
};
314316
} // namespace pcpp

Pcap++/src/XdpDevice.cpp

Lines changed: 29 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -439,21 +439,16 @@ namespace pcpp
439439
return true;
440440
}
441441

442-
bool XdpDevice::initConfig()
442+
bool XdpDevice::populateConfigDefaults(XdpDeviceConfiguration& config) const
443443
{
444-
if (!m_Config)
445-
{
446-
m_Config = new XdpDeviceConfiguration();
447-
}
448-
449-
uint16_t numFrames = m_Config->umemNumFrames ? m_Config->umemNumFrames : DEFAULT_UMEM_NUM_FRAMES;
450-
uint16_t frameSize = m_Config->umemFrameSize ? m_Config->umemFrameSize : getpagesize();
451-
uint32_t fillRingSize = m_Config->fillRingSize ? m_Config->fillRingSize : DEFAULT_FILL_RING_SIZE;
444+
uint16_t numFrames = config.umemNumFrames ? config.umemNumFrames : DEFAULT_UMEM_NUM_FRAMES;
445+
uint16_t frameSize = config.umemFrameSize ? config.umemFrameSize : getpagesize();
446+
uint32_t fillRingSize = config.fillRingSize ? config.fillRingSize : DEFAULT_FILL_RING_SIZE;
452447
uint32_t completionRingSize =
453-
m_Config->completionRingSize ? m_Config->completionRingSize : DEFAULT_COMPLETION_RING_SIZE;
454-
uint32_t rxSize = m_Config->rxSize ? m_Config->rxSize : XSK_RING_CONS__DEFAULT_NUM_DESCS;
455-
uint32_t txSize = m_Config->txSize ? m_Config->txSize : XSK_RING_PROD__DEFAULT_NUM_DESCS;
456-
uint32_t batchSize = m_Config->rxTxBatchSize ? m_Config->rxTxBatchSize : DEFAULT_BATCH_SIZE;
448+
config.completionRingSize ? config.completionRingSize : DEFAULT_COMPLETION_RING_SIZE;
449+
uint32_t rxSize = config.rxSize ? config.rxSize : XSK_RING_CONS__DEFAULT_NUM_DESCS;
450+
uint32_t txSize = config.txSize ? config.txSize : XSK_RING_PROD__DEFAULT_NUM_DESCS;
451+
uint32_t batchSize = config.rxTxBatchSize ? config.rxTxBatchSize : DEFAULT_BATCH_SIZE;
457452

458453
if (frameSize != getpagesize())
459454
{
@@ -504,26 +499,38 @@ namespace pcpp
504499
return false;
505500
}
506501

507-
m_Config->umemNumFrames = numFrames;
508-
m_Config->umemFrameSize = frameSize;
509-
m_Config->fillRingSize = fillRingSize;
510-
m_Config->completionRingSize = completionRingSize;
511-
m_Config->rxSize = rxSize;
512-
m_Config->txSize = txSize;
513-
m_Config->rxTxBatchSize = batchSize;
502+
config.umemNumFrames = numFrames;
503+
config.umemFrameSize = frameSize;
504+
config.fillRingSize = fillRingSize;
505+
config.completionRingSize = completionRingSize;
506+
config.rxSize = rxSize;
507+
config.txSize = txSize;
508+
config.rxTxBatchSize = batchSize;
514509

515510
return true;
516511
}
517512

518513
bool XdpDevice::open()
514+
{
515+
return open(XdpDeviceConfiguration{});
516+
}
517+
518+
bool XdpDevice::open(const XdpDeviceConfiguration& config)
519519
{
520520
if (m_DeviceOpened)
521521
{
522522
PCPP_LOG_ERROR("Device already opened");
523523
return false;
524524
}
525525

526-
if (!(initConfig() && initUmem() &&
526+
auto configCopy = std::make_unique<XdpDeviceConfiguration>(config);
527+
if (!populateConfigDefaults(*configCopy))
528+
{
529+
return false;
530+
}
531+
m_Config = std::move(configCopy);
532+
533+
if (!(initUmem() &&
527534
populateFillRing(std::min(m_Config->fillRingSize, static_cast<uint32_t>(m_Config->umemNumFrames / 2))) &&
528535
configureSocket()))
529536
{
@@ -532,11 +539,7 @@ namespace pcpp
532539
delete m_Umem;
533540
m_Umem = nullptr;
534541
}
535-
if (m_Config)
536-
{
537-
delete m_Config;
538-
m_Config = nullptr;
539-
}
542+
m_Config.reset();
540543
return false;
541544
}
542545

@@ -547,12 +550,6 @@ namespace pcpp
547550
return m_DeviceOpened;
548551
}
549552

550-
bool XdpDevice::open(const XdpDeviceConfiguration& config)
551-
{
552-
m_Config = new XdpDeviceConfiguration(config);
553-
return open();
554-
}
555-
556553
void XdpDevice::close()
557554
{
558555
if (m_DeviceOpened)
@@ -561,7 +558,6 @@ namespace pcpp
561558
xsk_socket__delete(socketInfo->xsk);
562559
m_DeviceOpened = false;
563560
delete m_Umem;
564-
delete m_Config;
565561
m_Config = nullptr;
566562
m_Umem = nullptr;
567563
}

0 commit comments

Comments
 (0)